Preserve page resize information when main frame is remote

third_party/WebKit/Source/core/frame/VisualViewport.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 71 matching lines @@
 }
 
 DEFINE_TRACE(VisualViewport)
 {
     visitor->trace(m_frameHost);
     ScrollableArea::trace(visitor);
 }
 
 void VisualViewport::setSize(const IntSize& size)
 {
-    // When the main frame is remote, we won't have an associated frame.
-    if (!mainFrame())
-        return;
-
     if (m_size == size)
         return;
 
-    bool autosizerNeedsUpdating =
-        (size.width() != m_size.width())
+    bool autosizerNeedsUpdating = (size.width() != m_size.width())
+        && mainFrame()
         && mainFrame()->settings()
         && mainFrame()->settings()->textAutosizingEnabled();
 
     TRACE_EVENT2("blink", "VisualViewport::setSize", "width", size.width(), "height", size.height());
     m_size = size;
 
     if (m_innerViewportContainerLayer) {
         m_innerViewportContainerLayer->setSize(FloatSize(m_size));
 
         // Need to re-compute sizes for the overlay scrollbars.
@@ skipping 74 matching lines @@
 void VisualViewport::move(const FloatSize& delta)
 {
     setLocation(m_offset + delta);
 }
 
 void VisualViewport::setScale(float scale)
 {
     setScaleAndLocation(scale, m_offset);
 }
 
 void VisualViewport::setScaleAndLocation(float scale, const FloatPoint& location)

[dcheng, 2016/01/08 21:36:06]

What actually needs to run in here for a WebView with a remote main frame? Can
we do all the common work first and then early return halfway through?

[bokan, 2016/01/11 16:36:34]

Can a remote frame even be pinch-zoomed? I think maybe we don't need this method
to work for remote frames. e.g. If there's no frame than the clamping logic
below doesn't work (and will zero out the offset currently).

Perhaps the way this should work is that the root frame should get the pinch
gesture in its local process, then send some message to child frames' processes
where these values can be updated through some new method (e.g.
VisualViewport::updateValuesFromRemoteRoot)

[kenrb, 2016/01/11 17:07:24]

Pinch zoom doesn't really make sense on remote frames, so I think a lot of this
can be ignored, as Daniel suggests. I uploaded a new patchset that might make
more sense.

I think we need to preserve the scale here, though, because if the page scale
gets changed and then we swap the remote main frame back to a local main frame,
it probably needs to be correct in the same way as the size. Does that sound
right?

[bokan, 2016/01/11 18:18:58]

It sounds fine in principle but practically speaking, the only time we should be
swapping from remote to local is on a navigation right? That should always reset
the scale anyway.

My point earlier was that I don't think we should be changing the scale from a
remote frames' process at all. Pinch gestures should target the root frame and
other actions that cause zooming (such as focusing a text box on mobile) would
make more sense to send a message to the root and let it do the zoom. In that
case, we wouldn't have to worry about the VisualViewport's scale/location
changing in the remote process.

Anyway, I know this is all in flight so I'll stamp this change since it moves us
along. Just putting in my 2c for future direction :).

[kenrb, 2016/01/11 20:50:26]

I've removed this in the most recent patchset, since it isn't necessary to fix
current bugs and was speculative, that having stale values on VisualViewport
when a page is navigated in might be a problem in the same way as having a stale
size. Your comment makes it sounds like that is not likely to be possible, so it
is probably best to leave the method as is and investigate if we see any related
bugs pop up later on.

 {
-    if (!mainFrame())
-        return;
-
     bool valuesChanged = false;
 
     if (scale != m_scale) {
         m_scale = scale;
         valuesChanged = true;
         frameHost().chromeClient().pageScaleFactorChanged();
     }
 
     FloatPoint clampedOffset(clampOffsetToBoundaries(location));
 
     if (clampedOffset != m_offset) {
         m_offset = clampedOffset;
         scrollAnimator().setCurrentPosition(m_offset);
 
         // SVG runs with accelerated compositing disabled so no ScrollingCoordinator.
         if (ScrollingCoordinator* coordinator = frameHost().page().scrollingCoordinator())
             coordinator->scrollableAreaScrollLayerDidChange(this);
 
-        if (!frameHost().settings().inertVisualViewport()) {
+        if (!frameHost().settings().inertVisualViewport() && mainFrame()) {
             if (Document* document = mainFrame()->document())
                 document->enqueueScrollEventForNode(document);
         }
 
-        mainFrame()->loader().client()->didChangeScrollOffset();
+        if (mainFrame())
+            mainFrame()->loader().client()->didChangeScrollOffset();
         valuesChanged = true;
     }
 
     if (!valuesChanged)
         return;
 
-    InspectorInstrumentation::didUpdateLayout(mainFrame());
-    mainFrame()->loader().saveScrollState();
+    if (mainFrame()) {
+        InspectorInstrumentation::didUpdateLayout(mainFrame());
+        mainFrame()->loader().saveScrollState();
+    }
 
     clampToBoundaries();
 }
 
 bool VisualViewport::magnifyScaleAroundAnchor(float magnifyDelta, const FloatPoint& anchor)
 {
     const float oldPageScale = scale();
     const float newPageScale = frameHost().chromeClient().clampPageScaleFactorToLimits(
         magnifyDelta * oldPageScale);
     if (newPageScale == oldPageScale)
@@ skipping 497 matching lines @@
     } else if (graphicsLayer == m_rootTransformLayer) {
         name =  "Root Transform Layer";
     } else {
         ASSERT_NOT_REACHED();
     }
 
     return name;
 }
 
 } // namespace blink