Throw exception if page load fails

[baran, 2010-06-11 11:48:52]

Igor, shouldn't we be checking an environment variable to regard CURL warnings
as errors in WebDriver.

[Eran, 2010-06-11 12:43:02]

The major issue I saw is configuration of the driver through an environment
property - the other one is a small fix.

http://codereview.appspot.com/1651042/diff/1/5
File hlwk/WebKit/hl/JavaBindings.cpp (right):

http://codereview.appspot.com/1651042/diff/1/5#newcode109
hlwk/WebKit/hl/JavaBindings.cpp:109: static char* reportLoadErrors =
getenv("WEBKITDRIVER_REPORT_LOAD_ERRORS");
This should be configured on driver creation - it's a classic behavioural option
that should be supplied on driver creation and remain consistent during its
life.

http://codereview.appspot.com/1651042/diff/1/5#newcode330
hlwk/WebKit/hl/JavaBindings.cpp:330: String str =
client->error().localizedDescription();
What happens if the client getLoadingError was called when there was no error
and error() is null?

[viarheichyk, 2010-06-11 12:56:35]

http://codereview.appspot.com/1651042/diff/1/5
File hlwk/WebKit/hl/JavaBindings.cpp (right):

http://codereview.appspot.com/1651042/diff/1/5#newcode330
hlwk/WebKit/hl/JavaBindings.cpp:330: String str =
client->error().localizedDescription();
On 2010/06/11 12:43:03, Eran wrote:
> What happens if the client getLoadingError was called when there was no error
> and error() is null?

error() returns object, not pointer, so result will be empty string.

[baran, 2010-06-11 13:00:04]

On 11 June 2010 13:43, <eranm@google.com> wrote:

> The major issue I saw is configuration of the driver through an
> environment property - the other one is a small fix.
>
>
> http://codereview.appspot.com/1651042/diff/1/5
> File hlwk/WebKit/hl/JavaBindings.cpp (right):
>
> http://codereview.appspot.com/1651042/diff/1/5#newcode109
> hlwk/WebKit/hl/JavaBindings.cpp:109: static char* reportLoadErrors =
>
> getenv("WEBKITDRIVER_REPORT_LOAD_ERRORS");
> This should be configured on driver creation - it's a classic
> behavioural option that should be supplied on driver creation and remain
> consistent during its life.
>
Simon, Eran and I chatted about this a bit here, sounds like being able to
set this through the constructor or a setter is more sustainable. Can we
change to that?

>
> http://codereview.appspot.com/1651042/diff/1/5#newcode330
> hlwk/WebKit/hl/JavaBindings.cpp:330: String str =
> client->error().localizedDescription();
> What happens if the client getLoadingError was called when there was no
> error and error() is null?
>
>
> http://codereview.appspot.com/1651042/show
>

[simonstewart, 2010-06-14 10:02:55]

http://codereview.appspot.com/1651042/diff/1/5
File hlwk/WebKit/hl/JavaBindings.cpp (right):

http://codereview.appspot.com/1651042/diff/1/5#newcode109
hlwk/WebKit/hl/JavaBindings.cpp:109: static char* reportLoadErrors =
getenv("WEBKITDRIVER_REPORT_LOAD_ERRORS");
On 2010/06/11 12:43:03, Eran wrote:
> This should be configured on driver creation - it's a classic behavioural
option
> that should be supplied on driver creation and remain consistent during its
> life.

The "webdriver way" of doing this is to accept a "Capabilities" object in the
constructor, and to read the value of a well-known key from there. I suggest
adding the key as a public final static String on the WebKitDriver.

Naturally, there should also be a no-arg constructor, which should set sensible
defaults.