[webkit-reviews] review granted: [Bug 23626] Upstream null checks in Navigator.cpp : [Attachment 27493] patch v2

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Feng Qian <feng at chromium.org>'s
request for review:
Bug 23626: Upstream null checks in Navigator.cpp
https://bugs.webkit.org/show_bug.cgi?id=23626

Attachment 27493: patch v2
https://bugs.webkit.org/attachment.cgi?id=27493&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> +    // If the frame does not have a page, returns empty string instead of
> +    // calling FrameLoader::userAgent.
+    if (!m_frame->page())
+	 return String();
+	 
     return m_frame->loader()->userAgent(m_frame->document() ?
m_frame->document()->url() : KURL());

This is not a helpful comment - it says an extremely obvious thing about what
happens below, but doesn't explain the subtle reason. Also, I still think that
the check for document() can be removed.

> +  // In layout test mode, check one more time later.
> +  setTimeout(check_navigator_and_done, 200);

This comment is no longer true, and should be removed.

r=me, but comments will need to be tweaked when landing. Did I already say I
love this test case?