Commit 5eeba913 authored by wjmaclean's avatar wjmaclean Committed by Commit bot

Use IsRenderViewLive() to test for SadTab in ZoomController.

In a previous attempt to prevent SadTab pages from being zoomable we
attempted to use RenderProcessHost::HasConnection, but this fails in
the case where a new tab can be started in the same RenderProcessHost
while the SadTab is still visible. This CL switches to testing to see
if the RenderView for the tab in question is live in order to avoid
this problem.

BUG=407562

Review URL: https://codereview.chromium.org/529993002

Cr-Commit-Position: refs/heads/master@{#292988}
parent 560acd05
......@@ -4,6 +4,7 @@
#include "chrome/browser/ui/zoom/zoom_controller.h"
#include "chrome/browser/ui/sad_tab.h"
#include "chrome/browser/ui/zoom/zoom_event_manager.h"
#include "chrome/browser/ui/zoom/zoom_observer.h"
#include "content/public/browser/host_zoom_map.h"
......@@ -80,7 +81,7 @@ bool ZoomController::SetZoomLevelByExtension(
// Cannot zoom in disabled mode. Also, don't allow changing zoom level on
// a crashed tab.
if (zoom_mode_ == ZOOM_MODE_DISABLED ||
!web_contents()->GetRenderProcessHost()->HasConnection())
!web_contents()->GetRenderViewHost()->IsRenderViewLive())
return false;
// Store extension data so that |extension| can be attributed when the zoom
......
......@@ -13,6 +13,7 @@
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test_utils.h"
#include "testing/gmock/include/gmock/gmock.h"
......@@ -81,7 +82,7 @@ IN_PROC_BROWSER_TEST_F(ZoomControllerBrowserTest,
base::KillProcess(host->GetHandle(), 0, false);
crash_observer.Wait();
}
EXPECT_FALSE(web_contents->GetRenderProcessHost()->HasConnection());
EXPECT_FALSE(web_contents->GetRenderViewHost()->IsRenderViewLive());
// The following attempt to change the zoom level for a crashed tab should
// fail.
......
......@@ -14,6 +14,7 @@
#include "content/public/browser/host_zoom_map.h"
#include "content/public/browser/navigation_details.h"
#include "content/public/common/frame_navigate_params.h"
#include "content/public/test/test_renderer_host.h"
#include "content/public/test/test_utils.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -39,6 +40,11 @@ class ZoomControllerTest : public ChromeRenderViewHostTestHarness {
ChromeRenderViewHostTestHarness::SetUp();
zoom_controller_.reset(new ZoomController(web_contents()));
zoom_controller_->AddObserver(&zoom_observer_);
// This call is needed so that the RenderViewHost reports being alive. This
// is only important for tests that call ZoomController::SetZoomLevel().
content::RenderViewHostTester::For(rvh())->CreateRenderView(
base::string16(), MSG_ROUTING_NONE, MSG_ROUTING_NONE, -1, false);
}
virtual void TearDown() OVERRIDE {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment