[webkit-reviews] review denied: [Bug 87660] [BlackBerry] http authentication challenge issue when loading subresource : [Attachment 144357] Patch

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Jonathan Dong
<jonathan.dong at torchmobile.com.cn>'s request for review:
Bug 87660: [BlackBerry] http authentication challenge issue when loading
subresource
https://bugs.webkit.org/show_bug.cgi?id=87660

Attachment 144357: Patch
https://bugs.webkit.org/attachment.cgi?id=144357&action=review

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=144357&action=review


Looks good, but can maybe cleaned up some more.

> Source/WebCore/ChangeLog:14
> +	   No new tests because of no behavior changed.

No new tests since there is no change in behavior.

> Source/WebCore/platform/network/blackberry/NetworkJob.cpp:214
> +    // the saved credentials.

The comment is quite obvious, and the code below it is readable, maybe not
needed.

> Source/WebCore/platform/network/blackberry/NetworkJob.cpp:218
> +	   purgeCredentials();

Is it useful to log the fact that there was an error? How about dealing with
isError case first, you do not need the ! operation in that case.