Ensure zoom level is not updated if main frame is WebRemoteFrame.

When swapped out RenderFrame is not used, the RenderView/WebView does not have a main WebLocalFrame. This CL ensures the WebView code is not called in such cases until it is updated to properly handle out-of-process iframes. BUG=571603 Review URL: https://codereview.chromium.org/1546603003 Cr-Commit-Position: refs/heads/master@{#366698}
hylschwy · Dec 23, 2015 · eaa6adc · eaa6adc
1 parent f5fc0b0
commit eaa6adc
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 5 deletions.
diff --git a/content/public/test/render_view_test.cc b/content/public/test/render_view_test.cc
@@ -299,7 +299,7 @@ void RenderViewTest::TearDown() {
   scoped_ptr<blink::WebLeakDetector> leak_detector =
       make_scoped_ptr(blink::WebLeakDetector::create(this));
 
-  leak_detector->prepareForLeakDetection(GetMainFrame());
+  leak_detector->prepareForLeakDetection();
 
   view_ = NULL;
   mock_process_.reset();

diff --git a/content/renderer/render_view_browsertest.cc b/content/renderer/render_view_browsertest.cc
@@ -957,6 +957,30 @@ TEST_F(RenderViewImplTest, PaintAfterSwapOut) {
   new_view->Release();
 }
 
+// Verify that the renderer process doesn't crash when device scale factor
+// changes after a cross-process navigation has commited.
+// See https://crbug.com/571603.
+TEST_F(RenderViewImplTest, SetZoomLevelAfterCrossProcessNavigation) {
+  // This test should only run with out-of-process iframes enabled.
+  if (!SiteIsolationPolicy::AreCrossProcessFramesPossible())
+    return;
+
+  // The bug reproduces if zoom is used for devices scale factor.
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableUseZoomForDSF);
+
+  LoadHTML("Hello world!");
+
+  // Swap the main frame out after which it should become a WebRemoteFrame.
+  TestRenderFrame* main_frame =
+      static_cast<TestRenderFrame*>(view()->GetMainRenderFrame());
+  main_frame->SwapOut(kProxyRoutingId, true, content::FrameReplicationState());
+  EXPECT_TRUE(view()->webview()->mainFrame()->isWebRemoteFrame());
+
+  // This should not cause a crash.
+  view()->OnDeviceScaleFactorChanged();
+}
+
 // Test that we get the correct UpdateState message when we go back twice
 // quickly without committing.  Regression test for http://crbug.com/58082.
 // Disabled: http://crbug.com/157357 .

diff --git a/content/renderer/render_view_impl.h b/content/renderer/render_view_impl.h
@@ -567,6 +567,8 @@ class CONTENT_EXPORT RenderViewImpl
   FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest, SendCandidateWindowEvents);
   FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest, RenderFrameClearedAfterClose);
   FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest, PaintAfterSwapOut);
+  FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest,
+                           SetZoomLevelAfterCrossProcessNavigation);
   FRIEND_TEST_ALL_PREFIXES(RenderViewImplScaleFactorTest,
                            ConverViewportToScreenWithZoomForDSF);
   FRIEND_TEST_ALL_PREFIXES(RenderViewImplScaleFactorTest,

diff --git a/content/shell/renderer/layout_test/leak_detector.cc b/content/shell/renderer/layout_test/leak_detector.cc
@@ -60,7 +60,7 @@ LeakDetector::~LeakDetector() {
 }
 
 void LeakDetector::TryLeakDetection(blink::WebLocalFrame* frame) {
-  web_leak_detector_->prepareForLeakDetection(frame);
+  web_leak_detector_->prepareForLeakDetection();
   web_leak_detector_->collectGarbageAndReport();
 }
 

diff --git a/third_party/WebKit/Source/web/WebLeakDetector.cpp b/third_party/WebKit/Source/web/WebLeakDetector.cpp
@@ -62,7 +62,7 @@ WTF_MAKE_NONCOPYABLE(WebLeakDetectorImpl);
 
     ~WebLeakDetectorImpl() override {}
 
-    void prepareForLeakDetection(WebLocalFrame*) override;
+    void prepareForLeakDetection() override;
     void collectGarbageAndReport() override;
 
 private:
@@ -75,7 +75,7 @@ WTF_MAKE_NONCOPYABLE(WebLeakDetectorImpl);
     int m_numberOfGCNeeded;
 };
 
-void WebLeakDetectorImpl::prepareForLeakDetection(WebLocalFrame* frame)
+void WebLeakDetectorImpl::prepareForLeakDetection()
 {
     v8::Isolate* isolate = v8::Isolate::GetCurrent();
     v8::HandleScope handleScope(isolate);

diff --git a/third_party/WebKit/Source/web/WebViewImpl.cpp b/third_party/WebKit/Source/web/WebViewImpl.cpp
@@ -3079,6 +3079,11 @@ double WebViewImpl::setZoomLevel(double zoomLevel)
     else
         m_zoomLevel = zoomLevel;
 
+    // TODO(nasko): Setting zoom level needs to be refactored to support
+    // out-of-process iframes. See https://crbug.com/528407.
+    if (mainFrame()->isWebRemoteFrame())
+        return m_zoomLevel;
+
     LocalFrame* frame = mainFrameImpl()->frame();
     if (!WebLocalFrameImpl::pluginContainerFromFrame(frame)) {
         float zoomFactor = m_zoomFactorOverride ? m_zoomFactorOverride : static_cast<float>(zoomLevelToZoomFactor(m_zoomLevel));

diff --git a/third_party/WebKit/public/web/WebLeakDetector.h b/third_party/WebKit/public/web/WebLeakDetector.h
@@ -79,7 +79,7 @@ class WebLeakDetector {
 
     // Perform initial stage of preparing for leak detection,
     // releasing references to resources held globally.
-    virtual void prepareForLeakDetection(WebLocalFrame*) = 0;
+    virtual void prepareForLeakDetection() = 0;
 
     // Garbage collect Blink's heaps and report leak counts.
     // |WebLeakDetectorClient::onLeakDetectionComplete()| is called