Bug 113238

Summary: Non-paintsContent fixed position layer should not cause main-thread scrolling
Product: WebKit Reporter: Xianzhu Wang <wangxianzhu>
Component: Layout and RenderingAssignee: Xianzhu Wang <wangxianzhu>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, esprehn+autocc, jamesr, ojan.autocc, shawnsingh, simon.fraser, skyostil, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Xianzhu Wang 2013-03-25 13:38:06 PDT
Should record the reason of not-compositing non-paintsContent fixed position layer so that ScrollingCoordinator won't treat the layer as a fast-scrolling blocker.
Comment 1 Xianzhu Wang 2013-03-25 16:43:20 PDT
Created attachment 194946 [details]
Patch
Comment 2 Sami Kyöstilä 2013-03-26 04:16:32 PDT
Thanks Xianzhu, I think this looks great.
Comment 3 Xianzhu Wang 2013-03-26 12:09:45 PDT
Simon, could you please review the patch? Thanks.
Comment 4 James Robinson 2013-03-26 12:30:56 PDT
Comment on attachment 194946 [details]
Patch

R=me, this looks great.  See if you can get Simon to take a look as well before landing.
Comment 5 Simon Fraser (smfr) 2013-03-26 14:24:09 PDT
I think bug 110430 covers the same issue.
Comment 6 Xianzhu Wang 2013-03-26 14:53:44 PDT
Comment on attachment 194946 [details]
Patch

Assuming no objection from Simon :)
Comment 7 Simon Fraser (smfr) 2013-03-26 15:11:15 PDT
Comment on attachment 194946 [details]
Patch

Where is NotCompositedForNoVisibleContent actually consulted? I would expect to see a test for it in FrameView::scrollContentsFastPath(), but do not.
Comment 8 WebKit Review Bot 2013-03-26 15:14:31 PDT
Comment on attachment 194946 [details]
Patch

Clearing flags on attachment: 194946

Committed r146940: <http://trac.webkit.org/changeset/146940>
Comment 9 WebKit Review Bot 2013-03-26 15:14:34 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Simon Fraser (smfr) 2013-03-26 15:16:54 PDT
Please respond to my comment.
Comment 11 Xianzhu Wang 2013-03-26 15:24:49 PDT
(In reply to comment #7)
> (From update of attachment 194946 [details])
> Where is NotCompositedForNoVisibleContent actually consulted? I would expect to see a test for it in FrameView::scrollContentsFastPath(), but do not.

In ScrollingCoordinator::hasVisibleSlowRepaintViewportConstrainedObjects(), there is an existing test that any viewport constrained object not composited with explicit reasons (layer->viewportConstrainedNotCompositedReason() != RenderLayer::NoNotCompositedReason) won't cause main-thread scrolling.

Seems that we need the same test in FrameView::scrollContentsFastPath().
Comment 12 Xianzhu Wang 2013-03-26 15:32:39 PDT
(In reply to comment #11)
> (In reply to comment #7)
> > (From update of attachment 194946 [details] [details])
> > Where is NotCompositedForNoVisibleContent actually consulted? I would expect to see a test for it in FrameView::scrollContentsFastPath(), but do not.
> 
> In ScrollingCoordinator::hasVisibleSlowRepaintViewportConstrainedObjects(), there is an existing test that any viewport constrained object not composited with explicit reasons (layer->viewportConstrainedNotCompositedReason() != RenderLayer::NoNotCompositedReason) won't cause main-thread scrolling.
> 
> Seems that we need the same test in FrameView::scrollContentsFastPath().

This seems what bug 110430 covers. This bug covers the main-thread scrolling issue caused by no-content fixed element. I should have used "main-thread scrolling" instead of ambiguous "slow-scrolling"/"fast-scrolling". Will work on bug 110430 for the remaining issue in FrameView::scrollContentsFastPath().