[webkit-reviews] review granted: [Bug 72511] Make use-fixed-layout work reliable : [Attachment 115390] Patch (bug fix)

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Kenneth Rohde Christiansen
<kenneth at webkit.org>'s request for review:
Bug 72511: Make use-fixed-layout work reliable
https://bugs.webkit.org/show_bug.cgi?id=72511

Attachment 115390: Patch (bug fix)
https://bugs.webkit.org/attachment.cgi?id=115390&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=115390&action=review


r=me, but needs typo and comment fixes before landing :)

> Source/WebKit2/ChangeLog:13
> +	   as that wouldn't work in cases, such as when the web process has
> +	   crashes.

"has crashes" -> "has crashed" or simply "crashes" without has :)

>> Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:1144
>> +	m_frame->coreFrame()->createView(webPage->size(), backgroundColor,
false, IntSize(), isMainFrame && webPage->useFixedLayout());
> 
> maybe add a /* XXX */ comment to make explicit what this boolean is about?

I agree with Antonio, it would be nice to fix that when landing.

>> Source/WebKit2/WebProcess/WebPage/WebPage.h:266
>> +	bool useFixedLayout() const { return m_useFixedLayout; };
> 
> would usesFixedLayout or shouldUseFixedLayout read better?

useFixedLayout() is the same name that's also used for example in ScrollView.h.
I guess for the moment it's better to be consistent here.