Plumbing for frame-specific WebWidgets

Source/core/page/PageAnimator.cpp

Index: Source/core/page/PageAnimator.cpp
diff --git a/Source/core/page/PageAnimator.cpp b/Source/core/page/PageAnimator.cpp
index 0d429bae2ab329236e0351c91e2c33213606c226..a746d08d250338dd151dc18884f482b36baecfb2 100644
--- a/Source/core/page/PageAnimator.cpp
+++ b/Source/core/page/PageAnimator.cpp
@@ -66,12 +66,20 @@ void PageAnimator::serviceScriptedAnimations(double monotonicAnimationStartTime)
         documents[i]->serviceScriptedAnimations(monotonicAnimationStartTime);
 }
 
-void PageAnimator::scheduleVisualUpdate()
+void PageAnimator::scheduleVisualUpdate(LocalFrame* rootFrame)
 {
     // FIXME: also include m_animationFramePending here. It is currently not there due to crbug.com/353756.
     if (m_servicingAnimations || m_updatingLayoutAndStyleForPainting)
         return;
-    m_page->chrome().scheduleAnimation();
+    // FIXME: The frame-specific version of scheduleAnimation() is for
+    // out-of-process iframes. Passing 0 or the top-level frame to this method
+    // causes scheduleAnimation() to be called for the page, which still uses
+    // a page-level WebWidget (the WebViewImpl).
+    if (rootFrame && !rootFrame->isMainFrame() && rootFrame->isLocalRoot()) {

[dcheng, 2014/11/24 22:30:39]

Shouldn't we be able to assert rootFrame->isLocalRoot() if rootFrame isn't null?
Also, how hard would it be to get rid of this default arg and make sure we're
passing in the local root? I see just a few calls to this function in
Document/FrameSelection/RenderObject, and it's not clear to me that using the
main frame by default is actually correct.

[kenrb, 2014/11/25 17:14:56]

This goes to the WebViewImpl by default, which preserves existing behavior when
the main frame is local. For correct behavior with OOPIFs, PageAnimator needs to
be refactored.

[dcheng, 2014/11/25 20:05:13]

OK, can we still add an assert like ASSERT(!rootFrame ||
rootFrame->isLocalRoot())?

[kenrb, 2014/11/26 16:17:11]

No, it is called with non-root frames often. In that case it would probably be
more correct to find the local root and calling scheduleAnimationForFrame on
that, but since there is a FIXME and I am not sure how PageAnimator is going to
end up looking when mainFrame is remote, I have left it simpler for now.

+        m_page->chrome().scheduleAnimationForFrame(rootFrame);
+    } else {
+        m_page->chrome().scheduleAnimation();
+    }
 }
 
 void PageAnimator::updateLayoutAndStyleForPainting(LocalFrame* rootFrame)