Only scroll on main if the targeted frames need to scroll on main

third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp

Index: third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
diff --git a/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp b/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
index 185a3b5394f58aa92b4e2e7b321b828f831b107a..7cff1cd4da2f36e898cf0ddb1cf5a2554997f122 100644
--- a/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
+++ b/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
@@ -190,7 +190,7 @@ void ScrollingCoordinator::updateAfterCompositingChangeIfNeeded() {
   if (m_shouldScrollOnMainThreadDirty ||
       m_wasFrameScrollable != frameIsScrollable) {
     setShouldUpdateScrollLayerPositionOnMainThread(
-        mainThreadScrollingReasons());
+        mainThreadScrollingReasons(m_page->deprecatedLocalMainFrame()));
     m_shouldScrollOnMainThreadDirty = false;
   }
   m_wasFrameScrollable = frameIsScrollable;
@@ -1138,8 +1138,8 @@ bool ScrollingCoordinator::hasVisibleSlowRepaintViewportConstrainedObjects(
   return false;
 }
 
-MainThreadScrollingReasons ScrollingCoordinator::mainThreadScrollingReasons()
-    const {
+MainThreadScrollingReasons ScrollingCoordinator::mainThreadScrollingReasons(
+    LocalFrame* targetFrame) const {
   MainThreadScrollingReasons reasons =
       static_cast<MainThreadScrollingReasons>(0);
 
@@ -1149,22 +1149,12 @@ MainThreadScrollingReasons ScrollingCoordinator::mainThreadScrollingReasons()
   if (!m_page->mainFrame()->isLocalFrame())
     return reasons;
 
-  // TODO(flackr) Currently we combine reasons for main thread scrolling from
-  // all frames but we should only look at the targetted frame (and its
-  // ancestors if the scroll bubbles up). http://crbug.com/568901
-  for (Frame* frame = m_page->mainFrame(); frame;
-       frame = frame->tree().traverseNext()) {
-    if (!frame->isLocalFrame())

[flackr, 2016/12/02 19:24:02]

We should still continue if we hit a non local frame right?

[yigu, 2016/12/02 20:32:49]

Done.

-      continue;
-
-    // TODO(alexmos,kenrb): For OOPIF, local roots that are different from
-    // the main frame can't be used in the calculation, since they use
-    // different compositors with unrelated state, which breaks some of the
-    // calculations below.
-    if (toLocalFrame(frame)->localFrameRoot() != m_page->mainFrame())

[flackr, 2016/12/02 19:24:02]

Is this not still a concern with the new approach?

[yigu, 2016/12/02 20:32:49]

Done.

-      continue;
-
-    FrameView* frameView = toLocalFrame(frame)->view();
+  // Walk through the subtree from the target frame to root. Use the gathered
+  // reasons to determine whether the target frame should be scrolled on main
+  // thread regardless other subframes on the same page.
+  for (Frame* checkFrame = targetFrame; checkFrame;
+       checkFrame = checkFrame->tree().parent()) {
+    FrameView* frameView = toLocalFrame(checkFrame)->view();
     if (!frameView || frameView->shouldThrottleRendering())
       continue;
 
@@ -1189,15 +1179,38 @@ MainThreadScrollingReasons ScrollingCoordinator::mainThreadScrollingReasons()
           MainThreadScrollingReason::kHasNonLayerViewportConstrainedObjects;
     }
   }
+  if (reasons) {
+    // scrollLayer is not always created for iframes. When it is not,
+    // create one by enabling m_compositing.
+    if (WebLayer* scrollLayer =
+            toWebLayer(targetFrame->view()->layerForScrolling())) {
+      scrollLayer->addMainThreadScrollingReasons(reasons);
+    } else {
+      targetFrame->view()
+          ->layoutViewItem()
+          .compositor()
+          ->setCompositingModeEnabled(true);

[flackr, 2016/12/02 19:24:02]

I don't think we should force the creation of a scroll layer (or at least not in
this patch), we should just add the main thread scrolling reasons to the outer
scroll layer if the iframe doesn't have its own layer.

[yigu, 2016/12/02 20:32:49]

Deleted the scroll layer creation. Instead, I forced it by changing the
layouttest html file.

+      toWebLayer(targetFrame->view()->layerForScrolling())
+          ->addMainThreadScrollingReasons(reasons);
+    }
+  }
 
   return reasons;
 }
 
-String ScrollingCoordinator::mainThreadScrollingReasonsAsText() const {
-  DCHECK(m_page->deprecatedLocalMainFrame()->document()->lifecycle().state() >=
+String ScrollingCoordinator::mainThreadScrollingReasonsAsText(
+    Frame* frame) const {
+  DCHECK(toLocalFrame(frame)->document()->lifecycle().state() >=
          DocumentLifecycle::CompositingClean);
-  if (WebLayer* scrollLayer = toWebLayer(
-          m_page->deprecatedLocalMainFrame()->view()->layerForScrolling())) {
+
+  // If an iframe has attributes like background-attachment: fixed
+  // it should scroll on main thread. The reasons is added in
+  // mainThreadScrollingReasons. Need to call this function
+  // before access such reason.
+  mainThreadScrollingReasons(toLocalFrame(frame));

[flackr, 2016/12/02 19:24:02]

Shouldn't it have already been called / the reason added? If you have to call it
here it suggests that we may not have added the reason in a real world use case.

[yigu, 2016/12/02 20:32:49]

In some layout tests regarding iframe that has background-attachment:fixed,
mainThreadScrollingReasonsAsText may be called without
mainThreadScrollingReasons therefore the scrollLayer of the iframe is not
created. In those cases trying to access the reasons for an iframe will fail.

+
+  if (WebLayer* scrollLayer =
+          toWebLayer(toLocalFrame(frame)->view()->layerForScrolling())) {
     String result(MainThreadScrollingReason::mainThreadScrollingReasonsAsText(
                       scrollLayer->mainThreadScrollingReasons())
                       .c_str());