Bug 73128

Summary: Clean up code in WK2/ChromeClientClient related to viewport handling
Product: WebKit Reporter: Kenneth Rohde Christiansen <kenneth>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: hausmann, zalan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch hausmann: review+

Description Kenneth Rohde Christiansen 2011-11-25 05:21:25 PST 
1) Make sure that we only call resizeToContentsIfNeeded when we are using the tiled backing store and fixed layout as well.

2) Guard the scrollbar code so that it is only called when scroll delegation is not used. Do similarily for the frameset code, which makes no sense with frame flattening.

etc
Comment 1 Kenneth Rohde Christiansen 2011-11-25 05:22:34 PST 
Created attachment 116603 [details]
Patch
Comment 2 zalan 2011-11-25 05:43:06 PST 
lgtm. 
a small observation.
+            This method is only called for the main frame, so the main frame
+            check has been removed.
It looks to me a null check rather than a mainframe check. However, m_page->useFixedLayout() does eliminate the need for the null check, so the code is correct.
Comment 3 Simon Hausmann 2011-11-25 05:47:06 PST 
Comment on attachment 116603 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=116603&action=review

r=me but I'll let you decide on cq+ based on the #ifdef comment

> Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:448
> +#if USE(TILED_BACKING_STORE)
> +    if (m_page->useFixedLayout())
> +        m_page->resizeToContentsIfNeeded();
> +#endif

For WK2 it does seem a bit strange to have USE(TILED_BACKING_STORE) for this. Do we actually need the #ifdef?
Comment 4 Kenneth Rohde Christiansen 2011-11-25 05:53:24 PST 
Comment on attachment 116603 [details]
Patch

landed in r101167