The scroll and clip layers in WKCACFLayerRenderer are redundant now that RenderLayerCompositor has the same things (added for iframes, but also usable for Windows).
Created attachment 67874 [details] Patch
Attachment 67874 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/win/WebView.cpp:2096: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 67874 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=67874&action=prettypatch > WebCore/page/Frame.cpp:883 > + GraphicsLayer* rootLayer = compositor->layerTreeAsTextRootLayer(); The name of this call is very confusing. I know you're trying to say that this is the root layer you would use when calling layerTreeAsText, but that seems awfully specific. Why not just combine the calls, naming it something like 'rootLayerTreeAsText()' and have it return a string?
Comment on attachment 67874 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=67874&action=prettypatch > WebCore/rendering/RenderLayerCompositor.cpp:1298 > // FIXME: eventually we should do this on other platforms too. > if (!m_renderView->frameView()->platformWidget()) > return true; > -#endif > +#endif // PLATFORM(MAC) > + > return false; > +#endif // PLATFORM(WIN) Is Win vs. Mac the right distinction here? Why isn't the !m_renderView->frameView()->platformWidget() check sufficient?
(In reply to comment #4) > Is Win vs. Mac the right distinction here? Why isn't the !m_renderView->frameView()->platformWidget() check sufficient? On Windows we want to always return true. On Mac we want to return true for iframes in WebKit1, and in WebKit2. I think other platforms would be like windows.
(In reply to comment #5) > (In reply to comment #4) > > Is Win vs. Mac the right distinction here? Why isn't the !m_renderView->frameView()->platformWidget() check sufficient? > > On Windows we want to always return true. > On Mac we want to return true for iframes in WebKit1, and in WebKit2. > I think other platforms would be like windows. Right. What I mean is that we want to do those things due to other choices that have been made on each platform (e.g., Mac returns false for iframes in WebKit1 because it uses NSScrollViews for them (I think)), not because of something intrinsic to each platform. So it seems inappropriate to be using PLATFORM macros. We should instead be detecting whatever the choices were that we've made that require us to return a specific value. I.e., is there a way we can write this so that if we somehow were to change WebKit1 on Mac not to use NSScrollViews for iframes, the code would do the right thing automatically? And vice-versa for Windows?
Created attachment 67927 [details] Patch
Attachment 67927 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/win/WebView.cpp:2096: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 67927 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=67927&action=prettypatch r=me with style fix > WebKit/win/WebView.cpp:2096 > + if (lParam != 0) Style bot complained about this. Should be if (lParam)
http://trac.webkit.org/changeset/67735
Comment on attachment 67927 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=67927&action=review > WebCore/rendering/RenderLayerCompositor.cpp:1299 > + // We need to handle our own scrolling if we're: > + return !m_renderView->frameView()->platformWidget() // viewless (i.e. non-Mac, or Mac in WebKit2) > + || attachment == RootLayerAttachedViaEnclosingIframe; // a composited iframe on Mac Yay!