Enable resize and layout for frames with RemoteFrame ancestors

Source/web/WebViewImpl.cpp

Index: Source/web/WebViewImpl.cpp
diff --git a/Source/web/WebViewImpl.cpp b/Source/web/WebViewImpl.cpp
index 2aeac594bd5bc54bfca841b43a094195f446cce6..216b0100b3d54dccdc8ca74ef5f4c8d0dd860436 100644
--- a/Source/web/WebViewImpl.cpp
+++ b/Source/web/WebViewImpl.cpp
@@ -1589,12 +1589,26 @@ void WebViewImpl::resizePinchViewport(const WebSize& newSize)
     page()->frameHost().pinchViewport().setSize(newSize);
 }
 
+WebLocalFrameImpl* WebViewImpl::localFrameRootTemporary()
+{
+    // FIXME: This is a temporary method that finds the first localFrame in a traversal.
+    // This is equivalent to mainFrame() if the mainFrame is in-process. We need to create
+    // separate WebWidgets to be created by RenderWidgets, which are associated with *all*
+    // local frame roots, not just the first one in the tree. Until then, this limits us
+    // to having only one functioning connected LocalFrame subtree per process.
+    for (WebCore::Frame* frame = page()->mainFrame(); frame; frame = frame->tree().traverseNext()) {
+        if (frame->isLocalRoot())
+            return WebLocalFrameImpl::fromFrame(toLocalFrame(frame));
+    }
+    return 0;
+}
+
 void WebViewImpl::resize(const WebSize& newSize)
 {
     if (m_shouldAutoResize || m_size == newSize)
         return;
 
-    FrameView* view = mainFrameImpl()->frameView();
+    FrameView* view = localFrameRootTemporary()->frameView();
     if (!view)
         return;
 
@@ -1614,18 +1628,21 @@ void WebViewImpl::resize(const WebSize& newSize)
     }
 
     {
-        // Avoids unnecessary invalidations while various bits of state in FastTextAutosizer are updated.
-        FastTextAutosizer::DeferUpdatePageInfo deferUpdatePageInfo(page());
+        // FIXME: FastTextAutosizer does not yet support out-of-process frames.

[eseidel, 2014/07/11 16:33:03]

Might want to keep +pdr in the loop.

[pdr., 2014/07/11 17:14:52]

Nice catch eseidel!

@kenrb, please file a bug for this.

[kenrb, 2014/07/11 20:03:33]

Filed (https://code.google.com/p/chromium/issues/detail?id=393285), but it might
be worth mentioning that that bug might be open for a while because IIUC text
autosizing is a mobile feature, and out-of-process iframes on mobile are
considered a lower priority.

+        if (!mainFrameImpl()->frame()->isLocalFrame()) {

[skobes, 2014/07/11 19:05:28]

This check doesn't make sense in the context of the comment.  Why do we want to
construct the DeferUpdatePageInfo when the main frame is NOT local?

+            // Avoids unnecessary invalidations while various bits of state in FastTextAutosizer are updated.
+            FastTextAutosizer::DeferUpdatePageInfo deferUpdatePageInfo(page());

[skobes, 2014/07/11 19:05:28]

Putting this inside the brace scope of the 'if' makes it a nop.  Was that the
intent?

[kenrb, 2014/07/11 20:03:33]

Good catch, this is an error. The ! should not be in the condition.

[skobes, 2014/07/11 20:30:36]

The ! is one issue.  The other issue is that DeferUpdatePageInfo only "defers"
the update until its dtor is called.  So if there is no code after it in the
same scope, it will update immediately which defeats the purpose.

If you wanted it to apply only for local frames I think you would need to pull
everything from here to the } on line 1659 into a helper function and do
something like

  if (mainFrameImpl()->frame()->isLocalFrame()) {
    FastTextAutosizer::DeferUpdatePageInfo deferUpdatePageInfo(page());
    performResize();
  } else {
    performResize();
  }

But, a higher level concern: DeferUpdatePageInfo is just a performance
optimization.  Removing it doesn't mean we don't autosize, it just means the
autosizer thrashes more.  The problem that DeferUpdatePageInfo solves is where

1. WebViewImpl says "hey your frame width is now 600 instead of 800..."
2. FTA says "oh noes! autosize all the things"
3. WebViewImpl says "... and also your LAYOUT width is 600"
4. FTA says "lol just kidding no autosizing pls"

If the intent is to *suppress* autosizing in OOP iframes, then you want to add a
check inside FTA::updatePageInfo, where it uses
m_document->page()->deprecatedLocalMainFrame().

[dcheng, 2014/07/11 21:22:52]

I thought fast text-autosizing only ran on mobile? I made several changes to
FastTextAutosizer which will should crash in the presence of RemoteFrames, but
those changes have not blocked us on desktop
(https://codereview.chromium.org/336553003). This particular location does,
because constructing the defer info always treats the main frame as local, but
it's not with OOPI. We don't actually really care if autosizing works at all in
the presence of RemoteFrames; we just care that it doesn't crash while we
bootstrap the architecture.

+        }
 
         m_pageScaleConstraintsSet.didChangeViewSize(m_size);
 
-        updatePageDefinedViewportConstraints(mainFrameImpl()->frame()->document()->viewportDescription());
+        updatePageDefinedViewportConstraints(localFrameRootTemporary()->frame()->document()->viewportDescription());
         updateMainFrameLayoutSize();
 
         // If the virtual viewport pinch mode is enabled, the main frame will be resized
         // after layout so it can be sized to the contentsSize.
-        if (!pinchVirtualViewportEnabled() && mainFrameImpl()->frameView())
-            mainFrameImpl()->frameView()->resize(m_size);
+        if (!pinchVirtualViewportEnabled() && localFrameRootTemporary()->frameView())
+            localFrameRootTemporary()->frameView()->resize(m_size);
 
         if (pinchVirtualViewportEnabled())
             page()->frameHost().pinchViewport().setSize(m_size);
@@ -1636,7 +1653,7 @@ void WebViewImpl::resize(const WebSize& newSize)
         // and thus will not be invalidated in |FrameView::performPreLayoutTasks|.
         // Therefore we should force explicit media queries invalidation here.
         if (page()->inspectorController().deviceEmulationEnabled()) {
-            if (Document* document = mainFrameImpl()->frame()->document())
+            if (Document* document = localFrameRootTemporary()->frame()->document())
                 document->mediaQueryAffectingValueChanged();
         }
     }
@@ -1735,7 +1752,7 @@ void WebViewImpl::animate(double monotonicFrameBeginTime)
 void WebViewImpl::layout()
 {
     TRACE_EVENT0("blink", "WebViewImpl::layout");
-    PageWidgetDelegate::layout(m_page.get());
+    PageWidgetDelegate::layout(m_page.get(), localFrameRootTemporary()->frame());
     updateLayerTreeBackgroundColor();
 
     for (size_t i = 0; i < m_linkHighlights.size(); ++i)