[webkit-reviews] review requested: [Bug 106142] Promote composited-scrolling layers to stacking containers. : [Attachment 184804] Patch

[bugzilla-daemon at webkit.org]

vollick at chromium.org has asked	for review:
Bug 106142: Promote composited-scrolling layers to stacking containers.
https://bugs.webkit.org/show_bug.cgi?id=106142

Attachment 184804: Patch
https://bugs.webkit.org/attachment.cgi?id=184804&action=review

------- Additional Comments from vollick at chromium.org
(In reply to comment #18)
  > (From update of attachment 184388 [details])
  > View in context:
https://bugs.webkit.org/attachment.cgi?id=184388&action=review
  > 
  > > Source/WebCore/rendering/RenderLayer.cpp:1926
  > > +        RenderLayer* sc = parent();
  > 
  > no abbreviations! 'sc' needs a better name
  > 
  > > Source/WebCore/rendering/RenderLayer.cpp:1933
  > > +        while (sc && !sc->isStackingContext())
  > > + 	   sc = sc->parent();
  > > +        
  > > +        if (sc) {
    > > +	     sc->dirtyZOrderLists();
    > > +	     sc->dirtyNormalFlowList();
    > > +	 }
    > 
    > why is this different from dirtyStackingContainerZOrderLists() ? if this
RenderLayer is in a stacking container that's not a stacking context's lists,
wouldn't we have to dirty those?
Good point! I've switched this to dirtyStackingContainerZOrderLists(). This
also fixes the variable naming problem you mentioned.
    > 
    > > Source/WebCore/rendering/RenderLayer.cpp:1935
    > > +	 compositor()->setCompositingLayersNeedRebuild();
    > > +	 compositor()->setShouldReevaluateCompositingAfterLayout();
    > 
    > why do you need both of these?
Unless I'm mistaken, if I just include the first, we will early out in
RenderLayerCompositor::updateCompositingLayers if we're not compositing (it
will think that there's no work to do), and if I include just the second, we
won't rebuild the layer tree if we're already compositing, and we'll need to if
we've dirtied a bunch of layer lists.