[webkit-reviews] review granted: [Bug 38827] FrameLoader: refactor changeLocation() and urlSelected() to share more code : [Attachment 56193] Proposed patch 2

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Chris Jerdonek
<cjerdonek at webkit.org>'s request for review:
Bug 38827: FrameLoader: refactor changeLocation() and urlSelected() to share
more code
https://bugs.webkit.org/show_bug.cgi?id=38827

Attachment 56193: Proposed patch 2
https://bugs.webkit.org/attachment.cgi?id=56193&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
LGTM.  Please consider these small tweaks below before landing.

WebCore/loader/FrameLoader.cpp:340
 +	bool lockHistory, bool lockBackForwardList, bool userGesture,
ReferrerPolicy referrerPolicy)
We usually don't add line breaks for function declarations, even when they're
ridiculously long like this one.

WebCore/loader/FrameLoader.cpp:352
 +	if (m_frame->script()->executeIfJavaScriptURL(request.url(),
userGesture, shouldReplaceDocumentIfJavaScriptURL ==
ReplaceDocumentIfJavaScriptURL))
You might consider propagating the enum down into executeIfJavaScriptURL so we
don't have to convert here and other callers (if there are any) will look
prettier.