Bug 73128 - Clean up code in WK2/ChromeClientClient related to viewport handling
: Clean up code in WK2/ChromeClientClient related to viewport handling
Status: RESOLVED FIXED
: WebKit
WebKit2
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2011-11-25 05:21 PST by
Modified: 2011-11-25 05:53 PST (History)


Attachments
Patch (6.46 KB, patch)
2011-11-25 05:22 PST, Kenneth Rohde Christiansen
hausmann: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 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 From 2011-11-25 05:22:34 PST -------
Created an attachment (id=116603) [details]
Patch
------- Comment #2 From 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 From 2011-11-25 05:47:06 PST -------
(From update of attachment 116603 [details])
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 From 2011-11-25 05:53:24 PST -------
(From update of attachment 116603 [details])
landed in r101167