Move bloated renderer reloading logic to BloatedRendererTabHelper.

Currently resource_coordinator::TabLifecycleUnit controls reloading
of a bloated renderer process and notifies the tab helper.

This CL moves the renderer process reloading to the tab helper, so
that all code related to bloated rendere is in one place.

Bug: 835806,808143
Change-Id: I1577872a1adc043a87bb2f599b211d3be9708f5c
Reviewed-on: https://chromium-review.googlesource.com/1097481
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569588}

chrome/browser/resource_coordinator/tab_lifecycle_unit.cc

@@ -142,19 +142,6 @@ StateChangeReason DiscardReasonToStateChangeReason(DiscardReason reason) {
   }
 }
 
-// These values are persisted to logs. Entries should not be renumbered and
-// numeric values should never be reused.
-enum class BloatedRendererHandlingInBrowser {
-  kReloaded = 0,
-  kCannotReload = 1,
-  kCannotShutdown = 2,
-  kMaxValue = kCannotShutdown
-};
-
-void RecordBloatedRendererHandling(BloatedRendererHandlingInBrowser handling) {
-  UMA_HISTOGRAM_ENUMERATION("BloatedRenderer.HandlingInBrowser", handling);
-}
-
 }  // namespace
 
 TabLifecycleUnitSource::TabLifecycleUnit::TabLifecycleUnit(
@@ -677,54 +664,6 @@ bool TabLifecycleUnitSource::TabLifecycleUnit::DiscardTab() {
   return Discard(DiscardReason::kExternal);
 }
 
-bool TabLifecycleUnitSource::TabLifecycleUnit::CanReloadBloatedTab() {
-  // Can't reload a tab that isn't in a TabStripModel, which is needed for
-  // showing an infobar.
-  if (!tab_strip_model_)
-    return false;
-
-  if (GetWebContents()->IsCrashed())
-    return false;
-
-  // Do not reload tabs that don't have a valid URL (most probably they have
-  // just been opened and discarding them would lose the URL).
-  if (!GetWebContents()->GetLastCommittedURL().is_valid() ||
-      GetWebContents()->GetLastCommittedURL().is_empty()) {
-    return false;
-  }
-
-  // Do not reload tabs in which the user has entered text in a form.
-  if (GetWebContents()->GetPageImportanceSignals().had_form_interaction)
-    return false;
-
-  // TODO(ulan): Check if the navigation controller has POST data.
-
-  return true;
-}
-
-void TabLifecycleUnitSource::TabLifecycleUnit::ReloadBloatedTab() {
-  if (CanReloadBloatedTab()) {
-    const size_t expected_page_count = 1u;
-    const bool skip_unload_handlers = true;
-    if (GetRenderProcessHost()->FastShutdownIfPossible(expected_page_count,
-                                                       skip_unload_handlers)) {
-      // TODO(ulan): Notify the WebContents that the page is bloated to give
-      // it a chance to show the infobar after the reload.
-      const bool check_for_repost = true;
-      GetWebContents()->GetController().Reload(content::ReloadType::NORMAL,
-                                               check_for_repost);
-      RecordBloatedRendererHandling(
-          BloatedRendererHandlingInBrowser::kReloaded);
-    } else {
-      RecordBloatedRendererHandling(
-          BloatedRendererHandlingInBrowser::kCannotShutdown);
-    }
-  } else {
-    RecordBloatedRendererHandling(
-        BloatedRendererHandlingInBrowser::kCannotReload);
-  }
-}
-
 bool TabLifecycleUnitSource::TabLifecycleUnit::IsDiscarded() const {
   // External code does not need to know about the intermediary PENDING_DISCARD
   // state. To external callers, the tab is discarded while in the

chrome/browser/resource_coordinator/tab_lifecycle_unit.h

@@ -5,7 +5,6 @@
 #ifndef CHROME_BROWSER_RESOURCE_COORDINATOR_TAB_LIFECYCLE_UNIT_H_
 #define CHROME_BROWSER_RESOURCE_COORDINATOR_TAB_LIFECYCLE_UNIT_H_
 
-#include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
 #include "base/time/time.h"
@@ -78,10 +77,6 @@ class TabLifecycleUnitSource::TabLifecycleUnit
   // unit.
   void UpdateLifecycleState(mojom::LifecycleState state);
 
-  // Reloads the tab because its renderer is bloated and shows an infobar
-  // explaining that it was reloaded because it ran out of memory.
-  void ReloadBloatedTab();
-
   // LifecycleUnit:
   TabLifecycleUnitExternal* AsTabLifecycleUnitExternal() override;
   base::string16 GetTitle() const override;
@@ -118,13 +113,6 @@ class TabLifecycleUnitSource::TabLifecycleUnit
   friend class TabLifecycleUnitSource;
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(TabLifecycleUnitTest, CanReloadBloatedTab);
-  FRIEND_TEST_ALL_PREFIXES(TabLifecycleUnitTest, CannotReloadBloatedTabCrashed);
-  FRIEND_TEST_ALL_PREFIXES(TabLifecycleUnitTest,
-                           CannotReloadBloatedTabInvalidURL);
-  FRIEND_TEST_ALL_PREFIXES(TabLifecycleUnitTest,
-                           CannotReloadBloatedTabPendingUserInteraction);
-
   // Determines if the tab is a media tab, and populates an optional
   // |decision_details| with full details.
   bool IsMediaTabImpl(DecisionDetails* decision_details) const;

chrome/browser/resource_coordinator/tab_lifecycle_unit_source.cc

@@ -229,11 +229,6 @@ void TabLifecycleUnitSource::OnLifecycleStateChanged(
     lifecycle_unit->UpdateLifecycleState(state);
 }
 
-void TabLifecycleUnitSource::OnRendererIsBloated(
-    content::WebContents* web_contents) {
-  GetTabLifecycleUnit(web_contents)->ReloadBloatedTab();
-}
-
 }  // namespace resource_coordinator
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(

chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h

@@ -114,7 +114,6 @@ class TabLifecycleUnitSource : public BrowserListObserver,
   // PageSignalObserver:
   void OnLifecycleStateChanged(content::WebContents* web_contents,
                                mojom::LifecycleState state) override;
-  void OnRendererIsBloated(content::WebContents* web_contents) override;
 
   // Tracks the BrowserList and all TabStripModels.
   BrowserTabStripTracker browser_tab_strip_tracker_;

chrome/browser/resource_coordinator/tab_lifecycle_unit_unittest.cc

@@ -334,39 +334,4 @@ TEST_F(TabLifecycleUnitTest, NotifiedOfWebContentsVisibilityChanges) {
   tab_lifecycle_unit.RemoveObserver(&observer);
 }
 
-TEST_F(TabLifecycleUnitTest, CanReloadBloatedTab) {
-  TabLifecycleUnit tab_lifecycle_unit(&observers_, web_contents_,
-                                      tab_strip_model_.get());
-  EXPECT_TRUE(tab_lifecycle_unit.CanReloadBloatedTab());
-}
-
-TEST_F(TabLifecycleUnitTest, CannotReloadBloatedTabCrashed) {
-  TabLifecycleUnit tab_lifecycle_unit(&observers_, web_contents_,
-                                      tab_strip_model_.get());
-
-  web_contents_->SetIsCrashed(base::TERMINATION_STATUS_PROCESS_CRASHED, 0);
-
-  EXPECT_FALSE(tab_lifecycle_unit.CanReloadBloatedTab());
-}
-
-TEST_F(TabLifecycleUnitTest, CannotReloadBloatedTabInvalidURL) {
-  TabLifecycleUnit tab_lifecycle_unit(&observers_, web_contents_,
-                                      tab_strip_model_.get());
-
-  content::WebContentsTester::For(web_contents_)
-      ->SetLastCommittedURL(GURL("invalid :)"));
-
-  EXPECT_FALSE(tab_lifecycle_unit.CanReloadBloatedTab());
-}
-
-TEST_F(TabLifecycleUnitTest, CannotReloadBloatedTabPendingUserInteraction) {
-  TabLifecycleUnit tab_lifecycle_unit(&observers_, web_contents_,
-                                      tab_strip_model_.get());
-  content::PageImportanceSignals signals;
-  signals.had_form_interaction = true;
-  content::WebContentsTester::For(web_contents_)
-      ->SetPageImportanceSignals(signals);
-  EXPECT_FALSE(tab_lifecycle_unit.CanReloadBloatedTab());
-}
-
 }  // namespace resource_coordinator

chrome/browser/resource_coordinator/tab_manager_browsertest.cc

@@ -266,12 +266,6 @@ class TabManagerTest : public InProcessBrowserTest {
         ->UpdateLifecycleState(mojom::LifecycleState::kFrozen);
   }
 
-  void ReloadBloatedTab(content::WebContents* contents) {
-    static_cast<TabLifecycleUnitSource::TabLifecycleUnit*>(
-        TabLifecycleUnitExternal::FromWebContents(contents))
-        ->ReloadBloatedTab();
-  }
-
   TabManager* tab_manager() { return g_browser_process->GetTabManager(); }
   TabStripModel* tsm() { return browser()->tab_strip_model(); }
 
@@ -1311,24 +1305,6 @@ IN_PROC_BROWSER_TEST_F(TabManagerTest,
   EXPECT_TRUE(IsTabDiscarded(browser4->tab_strip_model()->GetWebContentsAt(1)));
 }
 
-// TODO(ulan): Enable the test after fixing crbug.com/850921.
-IN_PROC_BROWSER_TEST_F(TabManagerTest, DISABLED_ReloadBloatedTab) {
-  content::WindowedNotificationObserver load(
-      content::NOTIFICATION_NAV_ENTRY_COMMITTED,
-      content::NotificationService::AllSources());
-  OpenURLParams url(GURL(chrome::kChromeUIAboutURL), content::Referrer(),
-                    WindowOpenDisposition::CURRENT_TAB,
-                    ui::PAGE_TRANSITION_TYPED, false);
-  browser()->OpenURL(url);
-  load.Wait();
-
-  content::WindowedNotificationObserver reload(
-      content::NOTIFICATION_NAV_ENTRY_COMMITTED,
-      content::NotificationService::AllSources());
-  ReloadBloatedTab(GetWebContentsAt(0));
-  reload.Wait();
-}
-
 }  // namespace resource_coordinator
 
 #endif  // OS_WIN || OS_MAXOSX || OS_LINUX || defined(OS_CHROMEOS)

chrome/browser/ui/bloated_renderer/bloated_renderer_tab_helper.cc

@@ -4,19 +4,41 @@
 
 #include "chrome/browser/ui/bloated_renderer/bloated_renderer_tab_helper.h"
 
+#include "base/metrics/histogram_macros.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/infobars/core/simple_alert_infobar_delegate.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/common/page_importance_signals.h"
 #include "ui/base/l10n/l10n_util.h"
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(BloatedRendererTabHelper);
 
+namespace {
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
+enum class BloatedRendererHandlingInBrowser {
+  kReloaded = 0,
+  kCannotReload = 1,
+  kCannotShutdown = 2,
+  kMaxValue = kCannotShutdown
+};
+
+void RecordBloatedRendererHandling(BloatedRendererHandlingInBrowser handling) {
+  UMA_HISTOGRAM_ENUMERATION("BloatedRenderer.HandlingInBrowser", handling);
+}
+}  // anonymous namespace
+
 BloatedRendererTabHelper::BloatedRendererTabHelper(
     content::WebContents* contents)
-    : content::WebContentsObserver(contents) {}
-
-void BloatedRendererTabHelper::WillReloadPageWithBloatedRenderer() {
-  reloading_bloated_renderer_ = true;
+    : content::WebContentsObserver(contents) {
+  auto* page_signal_receiver =
+      resource_coordinator::PageSignalReceiver::GetInstance();
+  if (page_signal_receiver) {
+    // PageSignalReceiver is not available if the resource coordinator is not
+    // enabled.
+    page_signal_receiver->AddObserver(this);
+  }
 }
 
 void BloatedRendererTabHelper::DidFinishNavigation(
@@ -29,6 +51,16 @@ void BloatedRendererTabHelper::DidFinishNavigation(
   }
 }
 
+void BloatedRendererTabHelper::WebContentsDestroyed() {
+  auto* page_signal_receiver =
+      resource_coordinator::PageSignalReceiver::GetInstance();
+  if (page_signal_receiver) {
+    // PageSignalReceiver is not available if the resource coordinator is not
+    // enabled.
+    page_signal_receiver->RemoveObserver(this);
+  }
+}
+
 void BloatedRendererTabHelper::ShowInfoBar(InfoBarService* infobar_service) {
   if (!infobar_service) {
     // No infobar service in unit-tests.
@@ -39,3 +71,52 @@ void BloatedRendererTabHelper::ShowInfoBar(InfoBarService* infobar_service) {
       infobars::InfoBarDelegate::BLOATED_RENDERER_INFOBAR_DELEGATE, nullptr,
       l10n_util::GetStringUTF16(IDS_BROWSER_BLOATED_RENDERER_INFOBAR), false);
 }
+
+bool BloatedRendererTabHelper::CanReloadBloatedTab() {
+  if (web_contents()->IsCrashed())
+    return false;
+
+  // Do not reload tabs that don't have a valid URL (most probably they have
+  // just been opened and discarding them would lose the URL).
+  if (!web_contents()->GetLastCommittedURL().is_valid() ||
+      web_contents()->GetLastCommittedURL().is_empty()) {
+    return false;
+  }
+
+  // Do not reload tabs in which the user has entered text in a form.
+  if (web_contents()->GetPageImportanceSignals().had_form_interaction)
+    return false;
+
+  // TODO(ulan): Check if the navigation controller has POST data.
+
+  return true;
+}
+
+void BloatedRendererTabHelper::OnRendererIsBloated(
+    content::WebContents* bloated_web_contents) {
+  if (web_contents() != bloated_web_contents) {
+    // Ignore if the notification is about a different tab.
+    return;
+  }
+  if (CanReloadBloatedTab()) {
+    const size_t expected_page_count = 1u;
+    const bool skip_unload_handlers = true;
+    content::RenderProcessHost* renderer =
+        web_contents()->GetMainFrame()->GetProcess();
+    if (renderer->FastShutdownIfPossible(expected_page_count,
+                                         skip_unload_handlers)) {
+      const bool check_for_repost = true;
+      reloading_bloated_renderer_ = true;
+      web_contents()->GetController().Reload(content::ReloadType::NORMAL,
+                                             check_for_repost);
+      RecordBloatedRendererHandling(
+          BloatedRendererHandlingInBrowser::kReloaded);
+    } else {
+      RecordBloatedRendererHandling(
+          BloatedRendererHandlingInBrowser::kCannotShutdown);
+    }
+  } else {
+    RecordBloatedRendererHandling(
+        BloatedRendererHandlingInBrowser::kCannotReload);
+  }
+}

chrome/browser/ui/bloated_renderer/bloated_renderer_tab_helper.h

@@ -7,42 +7,51 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "chrome/browser/resource_coordinator/page_signal_receiver.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 
 class InfoBarService;
 
-// This tab helper observes the WillReloadBloatedRenderer event. Upon
-// receiving the event, it activates the logic to show an infobar on the
+// This tab helper observes the OnRendererIsBloated event for its
+// WebContents. Upon receiving the event it reloads the bloated tab if
+// possible and activates the logic to show an infobar on the
 // subsequent DidFinishNavigation event.
 //
-// Assumptions around the WillReloadBloatedRenderer event:
-// - The renderer process was shutdown before it.
-// - Page reload will be performed immediately after it.
-// This ensures that the first DidFinishNavigation after it originates from
-// reloading the bloated page.
-//
 // Note that we need to show the infobar after NavigationEntryCommitted
 // because the infobar service removes existing infobars there.
 class BloatedRendererTabHelper
     : public content::WebContentsObserver,
-      public content::WebContentsUserData<BloatedRendererTabHelper> {
+      public content::WebContentsUserData<BloatedRendererTabHelper>,
+      public resource_coordinator::PageSignalObserver {
  public:
   ~BloatedRendererTabHelper() override = default;
 
   // content::WebContentsObserver:
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override;
+  void WebContentsDestroyed() override;
+
+  // resource_coordinator::PageSignalObserver:
+  void OnRendererIsBloated(content::WebContents* web_contents) override;
 
-  void WillReloadPageWithBloatedRenderer();
   static void ShowInfoBar(InfoBarService* infobar_service);
 
  private:
   friend class content::WebContentsUserData<BloatedRendererTabHelper>;
   FRIEND_TEST_ALL_PREFIXES(BloatedRendererTabHelperTest, DetectReload);
+  FRIEND_TEST_ALL_PREFIXES(BloatedRendererTabHelperTest, CanReloadBloatedTab);
+  FRIEND_TEST_ALL_PREFIXES(BloatedRendererTabHelperTest,
+                           CannotReloadBloatedTabCrashed);
+  FRIEND_TEST_ALL_PREFIXES(BloatedRendererTabHelperTest,
+                           CannotReloadBloatedTabInvalidURL);
+  FRIEND_TEST_ALL_PREFIXES(BloatedRendererTabHelperTest,
+                           CannotReloadBloatedTabPendingUserInteraction);
 
   explicit BloatedRendererTabHelper(content::WebContents* contents);
 
+  bool CanReloadBloatedTab();
+
   bool reloading_bloated_renderer_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(BloatedRendererTabHelper);

chrome/browser/ui/bloated_renderer/bloated_renderer_tab_helper_browsertest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/bloated_renderer/bloated_renderer_tab_helper.h"
+
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/infobars/infobar_service.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/url_constants.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_types.h"
+#include "content/public/test/test_navigation_observer.h"
+#include "content/public/test/test_utils.h"
+
+using BloatedRendererTabHelperBrowserTest = InProcessBrowserTest;
+
+// TODO(ulan): Enable the test after fixing crbug.com/850921.
+IN_PROC_BROWSER_TEST_F(BloatedRendererTabHelperBrowserTest,
+                       DISABLED_ReloadBloatedTab) {
+  content::WindowedNotificationObserver load(
+      content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+      content::NotificationService::AllSources());
+  content::OpenURLParams url(
+      GURL(chrome::kChromeUIAboutURL), content::Referrer(),
+      WindowOpenDisposition::CURRENT_TAB, ui::PAGE_TRANSITION_TYPED, false);
+  browser()->OpenURL(url);
+  load.Wait();
+
+  content::WindowedNotificationObserver reload(
+      content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+      content::NotificationService::AllSources());
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetWebContentsAt(0);
+  InfoBarService* infobar_service =
+      InfoBarService::FromWebContents(web_contents);
+  EXPECT_EQ(0u, infobar_service->infobar_count());
+  BloatedRendererTabHelper::FromWebContents(web_contents)
+      ->OnRendererIsBloated(web_contents);
+  reload.Wait();
+  EXPECT_EQ(1u, infobar_service->infobar_count());
+}

chrome/browser/ui/bloated_renderer/bloated_renderer_tab_helper_unittest.cc

@@ -4,17 +4,51 @@
 
 #include "chrome/browser/ui/bloated_renderer/bloated_renderer_tab_helper.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "content/public/common/page_importance_signals.h"
+#include "content/public/test/web_contents_tester.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-class BloatedRendererTabHelperTest : public ChromeRenderViewHostTestHarness {};
+class BloatedRendererTabHelperTest : public ChromeRenderViewHostTestHarness {
+ protected:
+  void SetUp() override {
+    ChromeRenderViewHostTestHarness::SetUp();
+    BloatedRendererTabHelper::CreateForWebContents(web_contents());
+    tab_helper_ = BloatedRendererTabHelper::FromWebContents(web_contents());
+    content::WebContentsTester::For(web_contents())
+        ->SetLastCommittedURL(GURL("https://test.test"));
+  }
+  BloatedRendererTabHelper* tab_helper_;
+};
 
 TEST_F(BloatedRendererTabHelperTest, DetectReload) {
-  BloatedRendererTabHelper::CreateForWebContents(web_contents());
-  BloatedRendererTabHelper* helper =
-      BloatedRendererTabHelper::FromWebContents(web_contents());
-  EXPECT_FALSE(helper->reloading_bloated_renderer_);
-  helper->WillReloadPageWithBloatedRenderer();
-  EXPECT_TRUE(helper->reloading_bloated_renderer_);
-  helper->DidFinishNavigation(nullptr);
-  EXPECT_FALSE(helper->reloading_bloated_renderer_);
+  EXPECT_FALSE(tab_helper_->reloading_bloated_renderer_);
+  tab_helper_->reloading_bloated_renderer_ = true;
+  tab_helper_->DidFinishNavigation(nullptr);
+  EXPECT_FALSE(tab_helper_->reloading_bloated_renderer_);
+}
+
+TEST_F(BloatedRendererTabHelperTest, CanReloadBloatedTab) {
+  EXPECT_TRUE(tab_helper_->CanReloadBloatedTab());
+}
+
+TEST_F(BloatedRendererTabHelperTest, CannotReloadBloatedTabCrashed) {
+  web_contents()->SetIsCrashed(base::TERMINATION_STATUS_PROCESS_CRASHED, 0);
+
+  EXPECT_FALSE(tab_helper_->CanReloadBloatedTab());
+}
+
+TEST_F(BloatedRendererTabHelperTest, CannotReloadBloatedTabInvalidURL) {
+  content::WebContentsTester::For(web_contents())
+      ->SetLastCommittedURL(GURL("invalid :)"));
+
+  EXPECT_FALSE(tab_helper_->CanReloadBloatedTab());
+}
+
+TEST_F(BloatedRendererTabHelperTest,
+       CannotReloadBloatedTabPendingUserInteraction) {
+  content::PageImportanceSignals signals;
+  signals.had_form_interaction = true;
+  content::WebContentsTester::For(web_contents())
+      ->SetPageImportanceSignals(signals);
+  EXPECT_FALSE(tab_helper_->CanReloadBloatedTab());
 }

chrome/test/BUILD.gn

@@ -716,6 +716,7 @@ test("browser_tests") {
       "../browser/sessions/tab_restore_browsertest.cc",
       "../browser/site_details_browsertest.cc",
       "../browser/thumbnails/thumbnail_browsertest.cc",
+      "../browser/ui/bloated_renderer/bloated_renderer_tab_helper_browsertest.cc",
       "../browser/ui/blocked_content/popup_tracker_browsertest.cc",
       "../browser/ui/blocked_content/safe_browsing_triggered_popup_blocker_browsertest.cc",
       "../browser/ui/blocked_content/tab_under_blocker_browsertest.cc",