Prevent Proactive tab discarding while offline.

Bug: 775644
Change-Id: I63764d060763c2040086c84ae02517f7bea3af52
Reviewed-on: https://chromium-review.googlesource.com/1132399Reviewed-by: François Doray <fdoray@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574135}

chrome/browser/resource_coordinator/tab_manager.cc

@@ -61,6 +61,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/page_importance_signals.h"
+#include "net/base/network_change_notifier.h"
 #include "third_party/blink/public/platform/web_sudden_termination_disabler_type.h"
 
 #if defined(OS_CHROMEOS)
@@ -997,8 +998,7 @@ void TabManager::PerformStateTransitions() {
   // oldest LifecycleUnit and call PerformStateTransitions() again, rather than
   // discarding all LifecycleUnits that have been non-visible for at least
   // GetTimeInBackgroundBeforeProactiveDiscard().
-  if (proactive_freeze_discard_params_.should_proactively_discard &&
-      oldest_discardable_lifecycle_unit) {
+  if (ShouldProactivelyDiscardTabs() && oldest_discardable_lifecycle_unit) {
     const base::TimeDelta time_not_visible =
         now - oldest_discardable_lifecycle_unit->GetLastActiveTime();
     const base::TimeDelta time_until_discard =
@@ -1086,4 +1086,15 @@ void TabManager::OnLifecycleUnitCreated(LifecycleUnit* lifecycle_unit) {
   SchedulePerformStateTransitions(base::TimeDelta());
 }
 
+bool TabManager::ShouldProactivelyDiscardTabs() {
+  if (!proactive_freeze_discard_params_.should_proactively_discard)
+    return false;
+
+  // Don't proactively discard tabs while offline.
+  if (net::NetworkChangeNotifier::IsOffline())
+    return false;
+
+  return true;
+}
+
 }  // namespace resource_coordinator

chrome/browser/resource_coordinator/tab_manager.h

@@ -434,6 +434,9 @@ class TabManager : public LifecycleUnitObserver,
   // LifecycleUnitSourceObserver:
   void OnLifecycleUnitCreated(LifecycleUnit* lifecycle_unit) override;
 
+  // Indicates if TabManager should proactively discard tabs.
+  bool ShouldProactivelyDiscardTabs();
+
   // LifecycleUnits managed by this.
   LifecycleUnitSet lifecycle_units_;
 

chrome/browser/resource_coordinator/tab_manager_unittest.cc

@@ -49,6 +49,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "content/public/test/web_contents_tester.h"
+#include "net/base/network_change_notifier.h"
 #include "services/resource_coordinator/public/cpp/resource_coordinator_features.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
@@ -111,6 +112,17 @@ enum TestIndicies {
   kInternalPage,
 };
 
+// Helper class to simulate being offline. NetworkChangeNotifier is a
+// singleton, making this instance is actually globally accessible. Users of
+// this class should first create a net::NetworkChangeNotifier::DisableForTest
+// object to allow the creation of this new NetworkChangeNotifier.
+class FakeOfflineNetworkChangeNotifier : public net::NetworkChangeNotifier {
+ public:
+  ConnectionType GetCurrentConnectionType() const override {
+    return NetworkChangeNotifier::CONNECTION_NONE;
+  }
+};
+
 }  // namespace
 
 class TabManagerTest : public testing::ChromeTestHarnessWithLocalDB {
@@ -131,7 +143,7 @@ class TabManagerTest : public testing::ChromeTestHarnessWithLocalDB {
         ->NavigateAndCommit(GURL("https://www.example.com"));
 
     base::RepeatingClosure run_loop_cb = base::BindRepeating(
-       &base::TestMockTimeTaskRunner::RunUntilIdle, task_runner_);
+        &base::TestMockTimeTaskRunner::RunUntilIdle, task_runner_);
 
     testing::WaitForLocalDBEntryToBeInitialized(web_contents.get(),
                                                 run_loop_cb);
@@ -1510,6 +1522,32 @@ TEST_F(TabManagerWithProactiveDiscardExperimentEnabledTest,
   tab_strip->CloseAllTabs();
 }
 
+TEST_F(TabManagerWithProactiveDiscardExperimentEnabledTest,
+       NoProactiveDiscardWhenOffline) {
+  auto window = std::make_unique<TestBrowserWindow>();
+  Browser::CreateParams params(profile(), true);
+  params.type = Browser::TYPE_TABBED;
+  params.window = window.get();
+  auto browser = std::make_unique<Browser>(params);
+  TabStripModel* tab_strip = browser->tab_strip_model();
+
+  // Simulate being offline.
+  net::NetworkChangeNotifier::DisableForTest disable_for_test;
+  FakeOfflineNetworkChangeNotifier fake_offline_state;
+
+  tab_strip->AppendWebContents(CreateWebContents(), /*foreground=*/true);
+  tab_strip->AppendWebContents(CreateWebContents(), /*foreground=*/false);
+  tab_strip->GetWebContentsAt(1)->WasShown();
+  tab_strip->GetWebContentsAt(1)->WasHidden();
+
+  task_runner_->FastForwardBy(kLowOccludedTimeout);
+
+  // The background tab shouldn't have been discarded while offline.
+  EXPECT_FALSE(IsTabDiscarded(tab_strip->GetWebContentsAt(1)));
+
+  tab_strip->CloseAllTabs();
+}
+
 TEST_F(TabManagerTest, NoProactiveDiscardWhenFeatureDisabled) {
   auto window = std::make_unique<TestBrowserWindow>();
   Browser::CreateParams params(profile(), true);