Bug 95494

Summary: Make RenderLayer::updateNeedsCompositedScrolling scrollbars agnostic
Product: WebKit Reporter: Antonio Gomes <tonikitoo>
Component: Layout and RenderingAssignee: Antonio Gomes <tonikitoo>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bdakin, dglazkov, eric, ojan.autocc, simon.fraser, skyostil, vollick, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 91117    
Attachments:
Description Flags
patch (for EWS)
webkit.review.bot: commit-queue-
patch
none
patch 1
webkit.review.bot: commit-queue-
patch 2 - fixed failing tests
webkit.review.bot: commit-queue-
patch 3 - slightly more conservative approach to fix the tests
webkit.review.bot: commit-queue-
patch 4 - materialization of comment #20
none
patch 4.1 - rebased against ToT after bug 94743
none
patch 4.2 - no const_cast jamesr: review+

Description Antonio Gomes 2012-08-30 13:19:26 PDT
This is always false for the Blackberry port, for example:

bool RenderLayer::allowsScrolling() const
{
    return (m_hBar && m_hBar->enabled()) || (m_vBar && m_vBar->enabled());
}

.....
We can query FrameView::ScrollableAreaSet instead
Comment 1 Antonio Gomes 2012-08-30 13:22:12 PDT
patch coming
Comment 2 Antonio Gomes 2012-08-30 13:33:11 PDT
Created attachment 161544 [details]
patch (for EWS)
Comment 3 WebKit Review Bot 2012-08-30 13:50:31 PDT
Comment on attachment 161544 [details]
patch (for EWS)

Attachment 161544 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13682856
Comment 4 Build Bot 2012-08-30 14:08:19 PDT
Comment on attachment 161544 [details]
patch (for EWS)

Attachment 161544 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13687827
Comment 5 Early Warning System Bot 2012-08-30 14:10:21 PDT
Comment on attachment 161544 [details]
patch (for EWS)

Attachment 161544 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13693872
Comment 6 Antonio Gomes 2012-08-30 14:11:07 PDT
Created attachment 161553 [details]
patch
Comment 7 Antonio Gomes 2012-12-12 08:32:30 PST
Getting back to this bug, finally...
Comment 8 Antonio Gomes 2012-12-12 09:19:32 PST
Created attachment 179063 [details]
patch 1
Comment 9 WebKit Review Bot 2012-12-12 10:04:36 PST
Comment on attachment 179063 [details]
patch 1

Attachment 179063 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15278637

New failing tests:
accessibility/disabled-controls-not-focusable.html
accessibility/canvas-fallback-content-2.html
accessibility/textarea-line-for-index.html
accessibility/canvas-accessibilitynodeobject.html
compositing/culling/unscrolled-within-boxshadow.html
http/tests/appcache/cyrillic-uri.html
accessibility/canvas-fallback-content.html
accessibility/aria-labelledby-on-input.html
accessibility/ellipsis-text.html
accessibility/crash-determining-aria-role-when-label-present.html
accessibility/aria-disabled.html
accessibility/title-ui-element-correctness.html
accessibility/aria-label.html
compositing/layers-inside-overflow-scroll.html
accessibility/aria-readonly.html
compositing/geometry/flipped-blocks-inline-mapping.html
compositing/self-painting-layers.html
compositing/geometry/geometry-map-scroll-during-layout-assertion.html
animations/animation-on-inline-crash.html
accessibility/secure-textfield-title-ui.html
compositing/geometry/limit-layer-bounds-overflow-root.html
accessibility/textarea-insertion-point-line-number.html
compositing/self-painting-layers2.html
compositing/geometry/foreground-offset-change.html
accessibility/aria-describedby-on-input.html
accessibility/aria-roles.html
accessibility/nested-layout-crash.html
compositing/culling/scrolled-within-boxshadow.html
compositing/masks/mask-of-clipped-layer.html
accessibility/textarea-selected-text-range.html
Comment 10 Antonio Gomes 2012-12-12 11:01:15 PST
Created attachment 179083 [details]
patch 2 - fixed failing tests
Comment 11 WebKit Review Bot 2012-12-12 11:59:45 PST
Comment on attachment 179083 [details]
patch 2 - fixed failing tests

Attachment 179083 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15286394

New failing tests:
css2.1/20110323/overflow-applies-to-007.htm
css2.1/20110323/dynamic-top-change-005b.htm
fast/block/positioning/absolute-in-inline-rtl.html
fast/block/positioning/absolute-in-inline-ltr-3.html
css2.1/20110323/inline-non-replaced-width-001.htm
fast/block/positioning/absolute-in-inline-ltr.html
http/tests/inspector/search/search-in-concatenated-script.html
http/tests/inspector/network/network-initiator-from-console.html
css2.1/20110323/inline-non-replaced-width-002.htm
css2.1/20110323/inline-box-002.htm
compositing/geometry/flipped-blocks-inline-mapping.html
css3/filters/filtered-inline.html
fast/block/positioning/absolute-in-inline-ltr-2.html
http/tests/inspector/search/search-in-script.html
compositing/overflow/get-transform-from-non-box-container.html
http/tests/inspector/network/request-parameters-decoding.html
fast/block/positioning/abs-inside-inline-rel.html
http/tests/inspector/network/network-sidebar-width.html
http/tests/inspector/compiler-source-mapping-debug.html
fast/block/positioning/058.html
fast/block/positioning/052.html
css2.1/20110323/dynamic-top-change-005.htm
compositing/transitions/opacity-on-inline.html
animations/animation-on-inline-crash.html
fast/block/positioning/absolute-in-inline-rtl-2.html
compositing/repaint/inline-repaint-container.html
fast/block/inline-children-root-linebox-crash.html
http/tests/inspector/network/network-preview-json.html
fast/block/positioning/absolute-in-inline-rtl-3.html
Comment 12 Simon Fraser (smfr) 2012-12-12 14:48:53 PST
Comment on attachment 179083 [details]
patch 2 - fixed failing tests

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

> Source/WebCore/ChangeLog:37
> +             (Pseudo-but-real-call-stack):
> +             RenderLayer::hasHorizontalOverflow()
> +             RenderLayer::hasScrollableHorizontalOverflow()
> +             RenderLayer::usesCompositedScrolling()
> +             RenderLayer::shouldBeNormalFlowOnly()
> +             RenderLayer::RenderLayer()
> +             RenderLayerModelObject::ensureLayer()
> +             RenderLayerModelObject::styleDidChange()
> +             RenderBox::styleDidChange()
> +             RenderBlock::styleDidChange()
> +             RenderObject::setStyle()
> +             RenderObject::setAnimatableStyle()
> +             NodeRenderingContext::createRendererForElementIfNeeded()
> +             Element::createRendererIfNeeded()
> +             Element::attach()

But given this call stack, how can you compute whether you're scrollable, since layout hasn't resized renderers yet? Or does it not matter because you'll layout and update soon anyway?
Comment 13 Antonio Gomes 2012-12-12 19:31:28 PST
(In reply to comment #12)
> (From update of attachment 179083 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179083&action=review
> 
> > Source/WebCore/ChangeLog:37
> > +             (Pseudo-but-real-call-stack):
> > +             RenderLayer::hasHorizontalOverflow()
> > +             RenderLayer::hasScrollableHorizontalOverflow()
> > +             RenderLayer::usesCompositedScrolling()
> > +             RenderLayer::shouldBeNormalFlowOnly()
> > +             RenderLayer::RenderLayer()
> > +             RenderLayerModelObject::ensureLayer()
> > +             RenderLayerModelObject::styleDidChange()
> > +             RenderBox::styleDidChange()
> > +             RenderBlock::styleDidChange()
> > +             RenderObject::setStyle()
> > +             RenderObject::setAnimatableStyle()
> > +             NodeRenderingContext::createRendererForElementIfNeeded()
> > +             Element::createRendererIfNeeded()
> > +             Element::attach()
> 
> But given this call stack, how can you compute whether you're scrollable, since layout hasn't resized renderers yet? Or does it not matter because you'll layout and update soon anyway?

I do not think it can be computed accurately indeed. I am trying to find a way to bail out when layer "is not ready", like above.

From my understanding, later on, a layout would finish and the layer would properly get promoted or not.
Comment 14 Antonio Gomes 2012-12-12 19:37:24 PST
(In reply to comment #11)
> (From update of attachment 179083 [details])
> Attachment 179083 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/15286394
> 
> New failing tests:
> css2.1/20110323/overflow-applies-to-007.htm
> css2.1/20110323/dynamic-top-change-005b.htm
> fast/block/positioning/absolute-in-inline-rtl.html
> fast/block/positioning/absolute-in-inline-ltr-3.html
> css2.1/20110323/inline-non-replaced-width-001.htm
> fast/block/positioning/absolute-in-inline-ltr.html
> http/tests/inspector/search/search-in-concatenated-script.html
> http/tests/inspector/network/network-initiator-from-console.html
> css2.1/20110323/inline-non-replaced-width-002.htm
> css2.1/20110323/inline-box-002.htm
> compositing/geometry/flipped-blocks-inline-mapping.html
> css3/filters/filtered-inline.html
> fast/block/positioning/absolute-in-inline-ltr-2.html
> http/tests/inspector/search/search-in-script.html
> compositing/overflow/get-transform-from-non-box-container.html
> http/tests/inspector/network/request-parameters-decoding.html
> fast/block/positioning/abs-inside-inline-rel.html
> http/tests/inspector/network/network-sidebar-width.html
> http/tests/inspector/compiler-source-mapping-debug.html
> fast/block/positioning/058.html
> fast/block/positioning/052.html
> css2.1/20110323/dynamic-top-change-005.htm
> compositing/transitions/opacity-on-inline.html
> animations/animation-on-inline-crash.html
> fast/block/positioning/absolute-in-inline-rtl-2.html
> compositing/repaint/inline-repaint-container.html
> fast/block/inline-children-root-linebox-crash.html
> http/tests/inspector/network/network-preview-json.html
> fast/block/positioning/absolute-in-inline-rtl-3.html

(In reply to comment #11)
> (From update of attachment 179083 [details])
> Attachment 179083 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/15286394
> 
> New failing tests:
> css2.1/20110323/overflow-applies-to-007.htm
> css2.1/20110323/dynamic-top-change-005b.htm
> fast/block/positioning/absolute-in-inline-rtl.html
> fast/block/positioning/absolute-in-inline-ltr-3.html
> css2.1/20110323/inline-non-replaced-width-001.htm
> fast/block/positioning/absolute-in-inline-ltr.html
> http/tests/inspector/search/search-in-concatenated-script.html
> http/tests/inspector/network/network-initiator-from-console.html
> css2.1/20110323/inline-non-replaced-width-002.htm
> css2.1/20110323/inline-box-002.htm
> compositing/geometry/flipped-blocks-inline-mapping.html
> css3/filters/filtered-inline.html
> fast/block/positioning/absolute-in-inline-ltr-2.html
> http/tests/inspector/search/search-in-script.html
> compositing/overflow/get-transform-from-non-box-container.html
> http/tests/inspector/network/request-parameters-decoding.html
> fast/block/positioning/abs-inside-inline-rel.html
> http/tests/inspector/network/network-sidebar-width.html
> http/tests/inspector/compiler-source-mapping-debug.html
> fast/block/positioning/058.html
> fast/block/positioning/052.html
> css2.1/20110323/dynamic-top-change-005.htm
> compositing/transitions/opacity-on-inline.html
> animations/animation-on-inline-crash.html
> fast/block/positioning/absolute-in-inline-rtl-2.html
> compositing/repaint/inline-repaint-container.html
> fast/block/inline-children-root-linebox-crash.html
> http/tests/inspector/network/network-preview-json.html
> fast/block/positioning/absolute-in-inline-rtl-3.html

These failures look real. I was able to get one of the stack traces (complete, and different from the previous one):

ASSERTION FAILED: !m_scrollDimensionsDirty
/home/agomes/OpenSource/webkit/Source/WebCore/rendering/RenderLayer.cpp(2648) : bool WebCore::RenderLayer::hasHorizontalOverflow() const
1   0xb2c2b6d9 WebCore::RenderLayer::hasHorizontalOverflow() const
2   0xb2c2b611 WebCore::RenderLayer::hasScrollableHorizontalOverflow() const
3   0xb2c26105 WebCore::RenderLayer::usesCompositedScrolling() const
4   0xb2c35cec WebCore::RenderLayer::shouldBeNormalFlowOnly() const
5   0xb2c219e0 WebCore::RenderLayer::RenderLayer(WebCore::RenderLayerModelObject*)
6   0xb2c40a69 WebCore::RenderLayerModelObject::ensureLayer()
7   0xb2c40fb9 WebCore::RenderLayerModelObject::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
8   0xb2ba5577 WebCore::RenderBox::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
9   0xb2b3e596 WebCore::RenderBlock::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
10  0xb2ca2301 WebCore::RenderTableCell::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
11  0xb2c6b40c WebCore::RenderObject::setStyle(WTF::PassRefPtr<WebCore::RenderStyle>)
12  0xb2c6adf9 WebCore::RenderObject::setAnimatableStyle(WTF::PassRefPtr<WebCore::RenderStyle>)
13  0xb24d61c8 WebCore::NodeRenderingContext::createRendererForElementIfNeeded()
14  0xb2473321 WebCore::Element::createRendererIfNeeded()
15  0xb247337e WebCore::Element::attach()
16  0xb26d1414
17  0xb26d1648 WebCore::HTMLConstructionSite::executeQueuedTasks()
18  0xb26f5436 WebCore::HTMLTreeBuilder::constructTreeFromAtomicToken(WebCore::AtomicHTMLToken*)
19  0xb26f5267 WebCore::HTMLTreeBuilder::constructTreeFromToken(WebCore::HTMLToken&)
20  0xb26d7013 WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode)
21  0xb26d6a7a WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode)
22  0xb26d75bd WebCore::HTMLDocumentParser::append(WebCore::SegmentedString const&)
23  0xb23ffab8 WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter*, char const*, unsigned int)
24  0xb28569fc WebCore::DocumentWriter::addData(char const*, unsigned int)
25  0xb28444c5 WebCore::DocumentLoader::commitData(char const*, unsigned int)
26  0xb7921035 WebKit::WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int)
27  0xb28442cb WebCore::DocumentLoader::commitLoad(char const*, int)
28  0xb28447b4 WebCore::DocumentLoader::receivedData(char const*, int)
29  0xb287db22 WebCore::MainResourceLoader::addData(char const*, int, bool)
30  0xb289142b WebCore::ResourceLoader::didReceiveData(char const*, int, long long, bool)
31  0xb287ee8c WebCore::MainResourceLoader::didReceiveData(char const*, int, long long, bool)

Program received signal SIGSEGV, Segmentation fault.
0xb2c2b6e3 in WebCore::RenderLayer::hasHorizontalOverflow (this=0x816d484)
    at /home/agomes/OpenSource/webkit/Source/WebCore/rendering/RenderLayer.cpp:2648
2648        ASSERT(!m_scrollDimensionsDirty);

Looking for a condition to best check within hasScrollable{Vertical,Horizontal}Overflow to bail out earlier when the layer is not ready like the above.
Comment 15 Antonio Gomes 2012-12-12 20:43:29 PST
Created attachment 179187 [details]
patch 3 - slightly more conservative approach to fix the tests
Comment 16 Simon Fraser (smfr) 2012-12-12 20:45:56 PST
You could possible check needsLayout() on the renderer.
Comment 17 WebKit Review Bot 2012-12-12 21:06:35 PST
Comment on attachment 179187 [details]
patch 3 - slightly more conservative approach to fix the tests

Attachment 179187 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15323075

New failing tests:
accessibility/disabled-controls-not-focusable.html
compositing/overflow/overflow-overlay-with-touch-no-overflow.html
accessibility/canvas-fallback-content-2.html
accessibility/textarea-line-for-index.html
compositing/overflow/overflow-clip-with-accelerated-scrolling-ancestor.html
compositing/culling/unscrolled-within-boxshadow.html
compositing/overflow/clipping-ancestor-with-accelerated-scrolling-ancestor.html
compositing/overflow/overflow-auto-with-touch-toggle.html
compositing/layer-creation/scroll-partial-update.html
compositing/overflow/overflow-scroll-with-touch-no-overflow.html
compositing/layers-inside-overflow-scroll.html
accessibility/aria-readonly.html
compositing/geometry/flipped-blocks-inline-mapping.html
compositing/self-painting-layers.html
compositing/overflow/scroll-ancestor-update.html
compositing/geometry/geometry-map-scroll-during-layout-assertion.html
animations/animation-on-inline-crash.html
compositing/overflow/overflow-auto-with-touch-no-overflow.html
compositing/overflow/remove-overflow-crash2.html
accessibility/textarea-insertion-point-line-number.html
compositing/overflow/remove-overflow-crash.html
compositing/overflow/get-transform-from-non-box-container.html
http/tests/inspector/compiler-source-mapping-debug.html
compositing/overflow/overflow-scroll.html
compositing/transitions/opacity-on-inline.html
accessibility/nested-layout-crash.html
compositing/culling/scrolled-within-boxshadow.html
compositing/repaint/inline-repaint-container.html
compositing/overflow/iframe-inside-overflow-clipping.html
accessibility/textarea-selected-text-range.html
Comment 18 Antonio Gomes 2012-12-13 11:56:21 PST
(In reply to comment #16)
> You could possible check needsLayout() on the renderer.

So, I've been trying a couple of conditions to catch when we should bail out before the layer is not ready to query the layer scrollability, including

-needsLayout on the renderer
-hasLayer on the renderer
-if the renderer has been parented (i.e. renderer()->parent())

And in all cases, the assert we reach occasionally is 

ASSERT(!m_scrollDimensionsDirty)

in RenderLayer::hasHorizontal/VerticalOverflow().

Given that, i could:

1) continue with this approach, and maybe call RenderLayer::computeScrollDimensions(). Problem here, could be a possible slightly performance loss.

2) return the layer as not scrollable if "scroll dimensions" is dirty. The problem here is that we can make a layer to toggle on/off needlessly its need for a graphics layer more often.

3) Change the approach: checking the containing frame view's scrollable area cache, if it contains this layer. No drawbacks I thought of.

Simon?
Comment 19 Simon Fraser (smfr) 2012-12-13 12:49:41 PST
You really have to delay this decision until layout is complete. We do that for some other things (see the requiresCompositingForPosition() code).
Comment 20 Antonio Gomes 2012-12-13 12:54:45 PST
(In reply to comment #19)
> You really have to delay this decision until layout is complete. We do that for some other things (see the requiresCompositingForPosition() code).

Approach #3 fits here, since a RenderLayer is cached by FrameView from either RenderLayer::updateScrollbarsAfterLayout or RenderLayer::updateScrollbarsAfterStyleChange:


....
 #if USE(ACCELERATED_COMPOSITING)
 bool RenderLayer::usesCompositedScrolling() const
 {
-    if (!scrollsOverflow() || !allowsScrolling())
+    FrameView* frameView = renderer()->view()->frameView();
+    if (!frameView || !frameView->containsScrollableArea(const_cast<RenderLayer*>(this)))
         return false;

 #if ENABLE(ACCELERATED_OVERFLOW_SCROLLING)
     return renderer()->style()->useTouchOverflowScrolling();
 #else
     return false;
 #endif
 }
 endif


Makes sense?
Comment 21 Antonio Gomes 2012-12-13 14:22:17 PST
Created attachment 179334 [details]
patch 4 - materialization of comment #20
Comment 22 Antonio Gomes 2012-12-13 22:02:37 PST
Comment on attachment 179334 [details]
patch 4 - materialization of comment #20

Raising the r? flag, although Simon suggested a different path on IRC, for the sake of this bug, querying if FrameView's scrollable area set cache (via ::containsScrollableArea) contains itself is enough, since FrameView::m_scrollableAreaSet is updated after every layout from RenderLayer::updateScrollbarsAfterLayout. 

That way, we are sure that the information gotten from FrameView::containsScrollableArea is in accordance with the last complete layout.

I can work on a follow up to convert this whole method to something similar to what other ::requiresCompositingForXXX method do with regards to "pending layout state".
Comment 23 Simon Fraser (smfr) 2012-12-13 22:07:05 PST
Comment on attachment 179334 [details]
patch 4 - materialization of comment #20

What are the implications of getting this wrong? For example, if in the previous layout the layer was scrollable (so it's in the scrollable area cache), but you're in the style change before a layout that will make it non-scrollable, is it OK to return true from usesCompositedScrolling()?
Comment 24 Antonio Gomes 2012-12-14 04:17:18 PST
(In reply to comment #23)
> (From update of attachment 179334 [details])
> What are the implications of getting this wrong? For example, if in the previous layout the layer was scrollable (so it's in the scrollable area cache), but you're in the style change before a layout that will make it non-scrollable, is it OK to return true from usesCompositedScrolling()?

if a style changes, making a previously scrollable layer not non-scrollable (say by setting overflow:hidden), then the call stack to notify the layer would be something like:

RenderLayer::styleChanged
RenderLayer::updateScrollbarsAfterStyleChange
RenderLayer::updateScrollableAreaSet <- FrameView gets up to date, and following calls to ::usesCompositedScrolling query the right value.

If the a layout actually finishes, the call stack would be:

RenderLayer::updateScrollInfoAfterLayout
RenderLayer::updateScrollbarsAfterLayout
RenderLayer::updateScrollableAreaSet <- FrameView gets up to date and following calls to ::usesCompositedScrolling query the right value.

So, from my understanding, it is a safe situation.

As least, it does not change the current behavior, since the methods that update the FrameView scrollable area set cache are the ones that also update the scrollbars (toggle them on/off).
Comment 25 Antonio Gomes 2012-12-14 04:19:04 PST
(In reply to comment #24)
> (In reply to comment #23)
> > (From update of attachment 179334 [details] [details])
> > What are the implications of getting this wrong? For example, if in the previous layout the layer was scrollable (so it's in the scrollable area cache), but you're in the style change before a layout that will make it non-scrollable, is it OK to return true from usesCompositedScrolling()?
> 
> if a style changes, making a previously scrollable layer not non-scrollable 

s/not non-scrollable/non-scrollable/g

(sorry about the noise)
Comment 26 Antonio Gomes 2012-12-17 13:47:18 PST
for the record:

<tonikitoo> smfr, hi. does https://bugs.webkit.org/show_bug.cgi?id=95494#c24 address your concerns?
<smfr> tonikitoo: i don't care about what the call stack looks like, but I care about whether the page is rendered correctly, or whether user interaction is broken somehow
<smfr> tonikitoo: e.g. I imagine we could have layers when we don't need them, but is that benign?
<tonikitoo> smfr, back. I understand your concern, and still I do not think that the patch changes the current behavior. As I said, I can work on a follow up (if needed) to address this.
<tonikitoo> smfr, i.e. this patch should not fix this bug, if it exists, but instead enable a few other configurations/ports to make use of what is already available
<tonikitoo> do you think this is  also a valid way to get it going?
<tonikitoo> smfr, answering your question (but is that benign?), as you said in the bug, if it triggers compositing on a layer right after a style change but still before the corresponding layout has finished (it will eventually), I do not see a  real world problem
<tonikitoo> but I would still stand, out of the scope of this bug
Comment 27 Antonio Gomes 2012-12-18 14:21:22 PST
Kindly ping reviewers.
Comment 28 Antonio Gomes 2012-12-19 03:56:33 PST
(In reply to comment #27)
> Kindly ping reviewers.

Added some potential reviewers.
Comment 29 Antonio Gomes 2012-12-19 12:14:44 PST
Created attachment 180205 [details]
patch 4.1 - rebased against ToT after bug 94743
Comment 30 Antonio Gomes 2012-12-20 10:33:56 PST
Simon, filed bug 105551 (Investigate the implications of RenderLayer::updateNeedsCompositedScrolling when there is a pending layout) to address any possible issue of this situation.
Comment 31 James Robinson 2013-01-04 10:32:38 PST
Comment on attachment 180205 [details]
patch 4.1 - rebased against ToT after bug 94743

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

> Source/WebCore/rendering/RenderLayer.cpp:1867
> +    if (!frameView || !frameView->containsScrollableArea(const_cast<RenderLayer*>(this)))

I still see a const_cast here
Comment 32 Antonio Gomes 2013-01-04 11:14:25 PST
Created attachment 181341 [details]
patch 4.2 - no const_cast


> > Source/WebCore/rendering/RenderLayer.cpp:1867
> > +    if (!frameView || !frameView->containsScrollableArea(const_cast<RenderLayer*>(this)))
> 
> I still see a const_cast here

Removed.
Comment 33 Antonio Gomes 2013-01-08 07:42:35 PST
Committed https://trac.webkit.org/changeset/139054