[webkit-reviews] review granted: [Bug 24992] crash at http://browserspy.dk/browser.php : [Attachment 30111] patch for FrameLoader::didOpenURL

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted robert
<robert at roberthogan.net>'s request for review:
Bug 24992: crash at http://browserspy.dk/browser.php
https://bugs.webkit.org/show_bug.cgi?id=24992

Attachment 30111: patch for FrameLoader::didOpenURL
https://bugs.webkit.org/attachment.cgi?id=30111&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> +	   Not sure how I would create a test case for this patch, sorry!
> +
> +	   * loader/FrameLoader.cpp:
> +	   (WebCore::FrameLoader::didOpenURL):

There should be a bug URL and title in the ChangeLog, so that one could easily
find the associated discussion. Also, it's best to describe changes for each
function - that's why the list of functions is generated.

As mentioned in a previous comment, we should have an ASSERT in
Frame::setJSStatusBarText(), so that accidental undoing of this fix (or other
similar issues) would be caught regardless of platform used. In fact, it would
be useful to add such assertions before other client calls.

r=me - I'll address my nitpicks while landing.