RC: Prevent discarding/freezing of tabs with an attached Devtools console.

Updated the privacy document (approved by msramek: https://docs.google.com/document/d/1BNQ5nLOtPuwP7oxr9r-XKNKr5iObXEiA_69WXAvuYAo/edit?usp=sharing)


Bug: 775644
Change-Id: If9e16711af2d27a7209366a61883997e865c9b50
Reviewed-on: https://chromium-review.googlesource.com/1126332Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573407}

chrome/browser/resource_coordinator/decision_details.cc

@@ -32,6 +32,7 @@ const char* kDecisionFailureReasonStrings[] = {
     "Tab is currently using WebSockets",
     "Tab is currently using WebUSB",
     "Tab is currently visible",
+    "Tab is currently using DevTools",
 };
 static_assert(base::size(kDecisionFailureReasonStrings) ==
                   static_cast<size_t>(DecisionFailureReason::MAX),
@@ -122,6 +123,9 @@ void PopulateFailureReason(
     case DecisionFailureReason::LIVE_STATE_VISIBLE:
       ukm->SetFailureLiveStateVisible(1);
       break;
+    case DecisionFailureReason::LIVE_STATE_DEVTOOLS_OPEN:
+      ukm->SetFailureLiveStateDevToolsOpen(1);
+      break;
     case DecisionFailureReason::MAX:
       break;
   }

chrome/browser/resource_coordinator/decision_details.h

@@ -68,6 +68,8 @@ enum class DecisionFailureReason : int32_t {
   LIVE_STATE_USING_WEB_USB,
   // The tab is opted out of the intervention as it is currently visible.
   LIVE_STATE_VISIBLE,
+  // The tab is opted out of the intervention as it's currently using DevTools.
+  LIVE_STATE_DEVTOOLS_OPEN,
   // This must remain last.
   MAX,
 };

chrome/browser/resource_coordinator/decision_details_unittest.cc

@@ -201,6 +201,8 @@ TEST(DecisionDetailsTest, TabManagerLifecycleStateChangeUkm) {
   EXPECT_FALSE(
       details.AddReason(DecisionFailureReason::LIVE_STATE_USING_WEB_USB));
   EXPECT_FALSE(details.AddReason(DecisionFailureReason::LIVE_STATE_VISIBLE));
+  EXPECT_FALSE(
+      details.AddReason(DecisionFailureReason::LIVE_STATE_DEVTOOLS_OPEN));
   EXPECT_TRUE(details.AddReason(
       DecisionSuccessReason::LIFECYCLES_FEATURE_POLICY_OPT_IN));
 
@@ -250,6 +252,8 @@ TEST(DecisionDetailsTest, TabManagerLifecycleStateChangeUkm) {
       entry, ukm_builder.kFailureLiveStateUsingWebUSBName, 1);
   ukm_recorder.ExpectEntryMetric(entry,
                                  ukm_builder.kFailureLiveStateVisibleName, 1);
+  ukm_recorder.ExpectEntryMetric(
+      entry, ukm_builder.kFailureLiveStateDevToolsOpenName, 1);
   EXPECT_FALSE(ukm_recorder.EntryHasMetric(
       entry, ukm_builder.kSuccessLifecyclesFeaturePolicyOptInName));
   EXPECT_FALSE(ukm_recorder.EntryHasMetric(

chrome/browser/resource_coordinator/tab_lifecycle_unit.cc

@@ -12,6 +12,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/optional.h"
 #include "base/process/process_metrics.h"
+#include "chrome/browser/devtools/devtools_window.h"
 #include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
 #include "chrome/browser/media/webrtc/media_stream_capture_indicator.h"
 #include "chrome/browser/profiles/profile.h"
@@ -425,21 +426,11 @@ bool TabLifecycleUnitSource::TabLifecycleUnit::CanFreeze(
     return false;
   }
 
-  // We deliberately run through all of the logic without early termination.
-  // This ensures that the decision details lists all possible reasons that the
-  // transition can be denied.
-
   if (GetWebContents()->GetVisibility() == content::Visibility::VISIBLE)
     decision_details->AddReason(DecisionFailureReason::LIVE_STATE_VISIBLE);
 
-  // Do not freeze tabs that are casting/mirroring/playing audio.
-  IsMediaTabImpl(decision_details);
-
-  // Consult the local database to see if this tab could try to communicate with
-  // the user while in background (don't check for the visibility here as
-  // there's already a check for that above).
-  CheckIfTabCanCommunicateWithUserWhileInBackground(GetWebContents(),
-                                                    decision_details);
+  CheckIfTabIsUsedInBackground(decision_details,
+                               false /* urgent_intervention */);
 
   if (decision_details->reasons().empty()) {
     decision_details->AddReason(
@@ -518,9 +509,6 @@ bool TabLifecycleUnitSource::TabLifecycleUnit::CanDiscard(
   if (GetWebContents()->GetPageImportanceSignals().had_form_interaction)
     decision_details->AddReason(DecisionFailureReason::LIVE_STATE_FORM_ENTRY);
 
-  // Do not discard tabs that are casting/mirroring/playing audio.
-  IsMediaTabImpl(decision_details);
-
   // Do not discard PDFs as they might contain entry that is not saved and they
   // don't remember their scrolling positions. See crbug.com/547286 and
   // crbug.com/65244.
@@ -534,13 +522,8 @@ bool TabLifecycleUnitSource::TabLifecycleUnit::CanDiscard(
         DecisionFailureReason::LIVE_STATE_EXTENSION_DISALLOWED);
   }
 
-  // Consult the local database to see if this tab could try to communicate with
-  // the user while in background (don't check for the visibility here as
-  // there's already a check for that above).
-  if (reason != DiscardReason::kUrgent) {
-    CheckIfTabCanCommunicateWithUserWhileInBackground(GetWebContents(),
-                                                      decision_details);
-  }
+  CheckIfTabIsUsedInBackground(decision_details,
+                               reason == DiscardReason::kUrgent);
 
   if (decision_details->reasons().empty()) {
     decision_details->AddReason(
@@ -817,4 +800,32 @@ void TabLifecycleUnitSource::TabLifecycleUnit::OnVisibilityChanged(
   OnLifecycleUnitVisibilityChanged(visibility);
 }
 
+void TabLifecycleUnitSource::TabLifecycleUnit::CheckIfTabIsUsedInBackground(
+    DecisionDetails* decision_details,
+    bool urgent_intervention) const {
+  DCHECK(decision_details);
+
+  // We deliberately run through all of the logic without early termination.
+  // This ensures that the decision details lists all possible reasons that the
+  // transition can be denied.
+
+  // Do not freeze tabs that are casting/mirroring/playing audio.
+  IsMediaTabImpl(decision_details);
+
+  // Consult the local database to see if this tab could try to communicate with
+  // the user while in background (don't check for the visibility here as
+  // there's already a check for that above). Skip this test if this is an
+  // urgent intervention.
+  if (!urgent_intervention) {
+    CheckIfTabCanCommunicateWithUserWhileInBackground(GetWebContents(),
+                                                      decision_details);
+  }
+
+  // Do not freeze tabs that are currently using DevTools.
+  if (DevToolsWindow::GetInstanceForInspectedWebContents(GetWebContents())) {
+    decision_details->AddReason(
+        DecisionFailureReason::LIVE_STATE_DEVTOOLS_OPEN);
+  }
+}
+
 }  // namespace resource_coordinator

chrome/browser/resource_coordinator/tab_lifecycle_unit.h

@@ -144,7 +144,13 @@ class TabLifecycleUnitSource::TabLifecycleUnit
   void DidStartLoading() override;
   void OnVisibilityChanged(content::Visibility visibility) override;
 
-  bool CanReloadBloatedTab();
+  // Indicates if freezing or discarding this tab would be noticeable by the
+  // user even if it isn't brought back to the foreground. Populates
+  // |decision_details| with full details. |urgent_intervention| indicates if
+  // this is for an urgent intervention, in which case some heuristics will be
+  // skipped.
+  void CheckIfTabIsUsedInBackground(DecisionDetails* decision_details,
+                                    bool urgent_intervention) const;
 
   // List of observers to notify when the discarded state or the auto-
   // discardable state of this tab changes.

chrome/browser/resource_coordinator/tab_manager.h

@@ -217,6 +217,7 @@ class TabManager : public LifecycleUnitObserver,
                            ProactiveFastShutdownWithBeforeunloadHandler);
   FRIEND_TEST_ALL_PREFIXES(TabManagerTest,
                            ProactiveFastShutdownWithUnloadHandler);
+  FRIEND_TEST_ALL_PREFIXES(TabManagerTest, ProtectDevToolsTabsFromDiscarding);
   FRIEND_TEST_ALL_PREFIXES(TabManagerTest, ProtectPDFPages);
   FRIEND_TEST_ALL_PREFIXES(TabManagerTest, ProtectRecentlyUsedTabs);
   FRIEND_TEST_ALL_PREFIXES(TabManagerTest, ProtectVideoTabs);

chrome/browser/resource_coordinator/tab_manager_browsertest.cc

@@ -12,6 +12,7 @@
 #include "base/test/simple_test_tick_clock.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/devtools/devtools_window_testing.h"
 #include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
 #include "chrome/browser/media/webrtc/media_stream_capture_indicator.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_unittest_utils.h"
@@ -605,6 +606,42 @@ IN_PROC_BROWSER_TEST_F(TabManagerTest, ProtectVideoTabs) {
   EXPECT_TRUE(tab_manager()->DiscardTabImpl(DiscardReason::kProactive));
 }
 
+// Makes sure that tabs using DevTools are protected from discarding.
+IN_PROC_BROWSER_TEST_F(TabManagerTest, ProtectDevToolsTabsFromDiscarding) {
+  // Get two tabs open, the second one being the foreground tab.
+  GURL test_page(ui_test_utils::GetTestUrl(
+      base::FilePath(), base::FilePath(FILE_PATH_LITERAL("simple.html"))));
+  ui_test_utils::NavigateToURL(browser(), test_page);
+  // Open a DevTools window for the first.
+  DevToolsWindow* devtool = DevToolsWindowTesting::OpenDevToolsWindowSync(
+      GetWebContentsAt(0), true /* is_docked */);
+  EXPECT_TRUE(devtool);
+
+  GURL url2(chrome::kChromeUIAboutURL);
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), GURL(chrome::kChromeUIAboutURL),
+      WindowOpenDisposition::NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // No discarding should be possible as the only background tab is currently
+  // using DevTools.
+  EXPECT_FALSE(tab_manager()->DiscardTabImpl(DiscardReason::kProactive));
+
+  // Close the DevTools window and repeat the test, this time use a non-docked
+  // window.
+  DevToolsWindowTesting::CloseDevToolsWindowSync(devtool);
+  devtool = DevToolsWindowTesting::OpenDevToolsWindowSync(
+      GetWebContentsAt(0), false /* is_docked */);
+  EXPECT_TRUE(devtool);
+  EXPECT_FALSE(tab_manager()->DiscardTabImpl(DiscardReason::kProactive));
+
+  // TODO(sebmarchand): Also ensure that the tab can't be frozen.
+
+  // Close the DevTools window, ensure that the tab can be discarded.
+  DevToolsWindowTesting::CloseDevToolsWindowSync(devtool);
+  EXPECT_TRUE(tab_manager()->DiscardTabImpl(DiscardReason::kProactive));
+}
+
 IN_PROC_BROWSER_TEST_F(TabManagerTest, CanPurgeBackgroundedRenderer) {
   // Open 2 tabs, the second one being in the background.
   ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIAboutURL));

tools/metrics/ukm/ukm.xml

@@ -3532,6 +3532,12 @@ be describing additional metrics about the same event.
       example).
     </summary>
   </metric>
+  <metric name="FailureLiveStateDevToolsOpen">
+    <summary>
+      Booling indicating that the intervention was disallowed because the tab is
+      currently using DevTools.
+    </summary>
+  </metric>
   <metric name="FailureLiveStateExtensionDisallowed">
     <summary>
       Booling indicating that the intervention was disallowed by an extension.