Add a timeout to the User Bypass reloading bubble

Adds a configurable timeout to the reloading bubble to prevent it from
showing for an extended period of time if a page takes a very long time
to load.

Bug: b:303071886
Change-Id: I7a12d8a10433fb4521a00d3d1f396890b206232f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4925330
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: Kevin Graney <kmg@google.com>
Cr-Commit-Position: refs/heads/main@{#1209009}

chrome/browser/ui/views/location_bar/cookie_controls/cookie_controls_bubble_view_controller.cc

@@ -23,6 +23,7 @@
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
+#include "cookie_controls_bubble_view_controller.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/ui_base_features.h"
 #include "ui/views/vector_icons.h"
@@ -90,8 +91,20 @@ void CookieControlsBubbleViewController::OnUserClosedContentView() {
   web_contents_->GetController().SetNeedsReload();
   web_contents_->GetController().LoadIfNecessary();
 
+  SwitchToReloadingView();
+}
+
+void CookieControlsBubbleViewController::SwitchToReloadingView() {
   bubble_view_->SwitchToReloadingView();
   bubble_view_->GetReloadingView()->RequestFocus();
+
+  // Set a timeout for how long the reloading view is shown for.
+  base::SingleThreadTaskRunner::GetCurrentDefault()->PostDelayedTask(
+      FROM_HERE,
+      base::BindOnce(
+          &CookieControlsBubbleViewController::OnReloadingViewTimeout,
+          weak_factory_.GetWeakPtr()),
+      content_settings::features::kUserBypassUIReloadBubbleTimeout.Get());
 }
 
 void CookieControlsBubbleViewController::OnFaviconFetched(
@@ -218,8 +231,23 @@ void CookieControlsBubbleViewController::OnBreakageConfidenceLevelChanged(
 
 void CookieControlsBubbleViewController::
     OnFinishedPageReloadWithChangedSettings() {
+  // TODO: Log a UserMetricsAction here to count completed page reloads once we
+  // have confidence that this callback is properly scoped.  See
+  // https://crrev.com/c/4925330 for context.
+  controller_observation_.Reset();
+  bubble_view_->CloseWidget();
+  // View destruction is call asynchronously from the bubble being closed, so we
+  // invalidate the weak pointers here to avoid callbacks happening after
+  // the bubble is closed and before this class is destroyed.
+  weak_factory_.InvalidateWeakPtrs();
+}
+
+void CookieControlsBubbleViewController::OnReloadingViewTimeout() {
+  base::RecordAction(
+      base::UserMetricsAction("CookieControls.Bubble.ReloadingTimeout"));
   controller_observation_.Reset();
   bubble_view_->CloseWidget();
+  weak_factory_.InvalidateWeakPtrs();
 }
 
 void CookieControlsBubbleViewController::SetCallbacks() {

chrome/browser/ui/views/location_bar/cookie_controls/cookie_controls_bubble_view_controller.h

@@ -53,6 +53,9 @@ class CookieControlsBubbleViewController
 
   void OnFaviconFetched(const favicon_base::FaviconImageResult& result) const;
 
+  void OnReloadingViewTimeout();
+
+  void SwitchToReloadingView();
   void ApplyThirdPartyCookiesAllowedState(base::Time expiration);
   void ApplyThirdPartyCookiesBlockedState();
 

chrome/browser/ui/views/location_bar/cookie_controls/cookie_controls_bubble_view_impl.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/views/location_bar/cookie_controls/cookie_controls_bubble_view_impl.h"
 
 #include <string>
+#include "base/metrics/user_metrics.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/accessibility/non_accessible_image_view.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
@@ -79,6 +80,8 @@ void CookieControlsBubbleViewImpl::UpdateFaviconImage(const gfx::Image& image,
 }
 
 void CookieControlsBubbleViewImpl::SwitchToReloadingView() {
+  base::RecordAction(
+      base::UserMetricsAction("CookieControls.Bubble.ReloadingShown"));
   GetReloadingView()->SetVisible(true);
   GetContentView()->SetVisible(false);
   SizeToContents();

chrome/browser/ui/views/location_bar/cookie_controls/cookie_controls_interactive_uitest.cc

@@ -42,6 +42,9 @@ const char kUMABubbleAllowThirdPartyCookies[] =
 const char kUMABubbleBlockThirdPartyCookies[] =
     "CookieControls.Bubble.BlockThirdPartyCookies";
 const char kUMABubbleSendFeedback[] = "CookieControls.Bubble.SendFeedback";
+const char kUMABubbleReloadingShown[] = "CookieControls.Bubble.ReloadingShown";
+const char kUMABubbleReloadingTimeout[] =
+    "CookieControls.Bubble.ReloadingTimeout";
 }  // namespace
 
 class CookieControlsInteractiveUiTest : public InteractiveBrowserTest {
@@ -57,7 +60,7 @@ class CookieControlsInteractiveUiTest : public InteractiveBrowserTest {
     iph_feature_list_.InitAndEnableFeatures(EnabledFeatures(),
                                             DisabledFeatures());
     https_server()->SetSSLConfig(net::EmbeddedTestServer::CERT_TEST_NAMES);
-    https_server()->ServeFilesFromSourceDirectory(GetChromeTestDataDir());
+    https_server()->AddDefaultHandlers(GetChromeTestDataDir());
 
     set_open_about_blank_on_browser_launch(true);
     ASSERT_TRUE(https_server()->InitializeAndListen());
@@ -179,9 +182,17 @@ class CookieControlsInteractiveUiTest : public InteractiveBrowserTest {
   content_settings::CookieSettings* cookie_settings() {
     return CookieSettingsFactory::GetForProfile(browser()->profile()).get();
   }
-  GURL third_party_cookie_page_url() {
-    return https_server()->GetURL("a.test",
-                                  "/third_party_partitioned_cookies.html");
+
+  // If slow is set to true will return a URL for a page that never finishes
+  // loading.
+  GURL third_party_cookie_page_url(bool slow = false) {
+    if (slow) {
+      return https_server()->GetURL(
+          "a.test", "/third_party_partitioned_cookies_slow.html");
+    } else {
+      return https_server()->GetURL("a.test",
+                                    "/third_party_partitioned_cookies.html");
+    }
   }
 
   static base::Time GetReferenceTime() {
@@ -441,6 +452,36 @@ IN_PROC_BROWSER_TEST_F(CookieControlsInteractiveUiTest, ReloadView) {
       WaitForHide(CookieControlsBubbleView::kCookieControlsBubble));
   EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleAllowThirdPartyCookies), 1);
   EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleBlockThirdPartyCookies), 0);
+  EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleReloadingTimeout), 0);
+  EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleReloadingShown), 1);
+}
+
+IN_PROC_BROWSER_TEST_F(CookieControlsInteractiveUiTest, ReloadViewTimeout) {
+  // Test that opening the bubble, then closing it after making a change,
+  // results in the reload view being displayed and then timing out.
+  //
+  // The page loaded in this test will never finish loading, so the timeout
+  // must be configured shorter than the test timeout.
+  BlockThirdPartyCookies();
+  RunTestSequence(
+      /*context(),*/ InstrumentTab(kWebContentsElementId),
+      EnterText(kOmniboxElementId,
+                base::UTF8ToUTF16(
+                    "https://" +
+                    third_party_cookie_page_url(/*slow=*/true).GetContent())),
+      Confirm(kOmniboxElementId),
+      InAnyContext(WaitForShow(kCookieControlsIconElementId)),
+      PressButton(kCookieControlsIconElementId),
+      InAnyContext(WaitForShow(CookieControlsBubbleView::kContentView)),
+      PressButton(CookieControlsContentView::kToggleButton),
+      PressButton(kLocationIconElementId),
+      InAnyContext(WaitForShow(CookieControlsBubbleView::kReloadingView)),
+      WaitForHide(CookieControlsBubbleView::kCookieControlsBubble));
+
+  EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleAllowThirdPartyCookies), 1);
+  EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleBlockThirdPartyCookies), 0);
+  EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleReloadingTimeout), 1);
+  EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleReloadingShown), 1);
 }
 
 IN_PROC_BROWSER_TEST_F(CookieControlsInteractiveUiTest,
@@ -484,6 +525,7 @@ IN_PROC_BROWSER_TEST_F(CookieControlsInteractiveUiTest,
       PressButton(kLocationIconElementId),
       EnsureNotPresent(CookieControlsBubbleView::kReloadingView),
       WaitForHide(CookieControlsBubbleView::kCookieControlsBubble));
+  EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleReloadingTimeout), 0);
 }
 
 IN_PROC_BROWSER_TEST_F(CookieControlsInteractiveUiTest,
@@ -527,6 +569,8 @@ IN_PROC_BROWSER_TEST_F(CookieControlsInteractiveUiTest,
       PressButton(kLocationIconElementId),
       InAnyContext(WaitForShow(CookieControlsBubbleView::kReloadingView)),
       WaitForHide(CookieControlsBubbleView::kCookieControlsBubble));
+  EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleReloadingTimeout), 0);
+  EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleReloadingShown), 1);
 }
 
 IN_PROC_BROWSER_TEST_F(CookieControlsInteractiveUiTest,
@@ -571,6 +615,8 @@ IN_PROC_BROWSER_TEST_F(CookieControlsInteractiveUiTest,
       PressButton(kLocationIconElementId),
       EnsureNotPresent(CookieControlsBubbleView::kReloadingView),
       WaitForHide(CookieControlsBubbleView::kCookieControlsBubble));
+  EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleReloadingTimeout), 0);
+  EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleReloadingShown), 0);
 }
 
 IN_PROC_BROWSER_TEST_F(CookieControlsInteractiveUiTest, NoReloadView) {
@@ -590,4 +636,6 @@ IN_PROC_BROWSER_TEST_F(CookieControlsInteractiveUiTest, NoReloadView) {
   EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleAllowThirdPartyCookies), 1);
   EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleBlockThirdPartyCookies), 1);
   EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleSendFeedback), 0);
+  EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleReloadingShown), 0);
+  EXPECT_EQ(user_actions_.GetActionCount(kUMABubbleReloadingTimeout), 0);
 }

chrome/test/data/third_party_partitioned_cookies_slow.html

@@ -0,0 +1,20 @@
+<html>
+  <head>
+    <title>cross-site iframes with partitioned cookies</title>
+    <script>
+      function setCookie() {
+        document.cookie = 'name=Good;';
+        console.log("onload");
+      }
+    </script>
+  </head>
+<body onload="setCookie();">
+<!-- This only works if the CrossSiteRedirector is running on the embedded test
+     server, and the host_resolver is set up to handle b.test and c.test. -->
+<iframe src="/cross-site/b.test/partitioned_cookie.html" id="frame1"></iframe>
+<iframe src="/cross-site/c.test/mixed_with_partitioned_cookie.html" id="frame2"></iframe>
+<iframe src="/cross-site/d.test/set_cookie_header.html" id="frame3"></iframe>
+<!-- The frame below will never finish loading.  Requires that EmbeddedTestServer
+     is started with default handlers. -->
+<iframe src="/hung-after-headers" />
+</body></html>

components/content_settings/core/common/features.cc

@@ -70,6 +70,9 @@ const base::FeatureParam<int> kUserBypassUIReloadCount{&kUserBypassUI,
 const base::FeatureParam<base::TimeDelta> kUserBypassUIReloadTime{
     &kUserBypassUI, "reload-time", base::Seconds(30)};
 
+const base::FeatureParam<base::TimeDelta> kUserBypassUIReloadBubbleTimeout{
+    &kUserBypassUI, "reload-bubble-timeout", base::Seconds(5)};
+
 BASE_FEATURE(kUserBypassFeedback,
              "UserBypassFeedback",
              base::FEATURE_ENABLED_BY_DEFAULT);

components/content_settings/core/common/features.h

@@ -93,6 +93,12 @@ extern const base::FeatureParam<int> kUserBypassUIReloadCount;
 COMPONENT_EXPORT(CONTENT_SETTINGS_FEATURES)
 extern const base::FeatureParam<base::TimeDelta> kUserBypassUIReloadTime;
 
+// The reloading bubble will be shown until either the page full reloads or this
+// timeout is reached.
+COMPONENT_EXPORT(CONTENT_SETTINGS_FEATURES)
+extern const base::FeatureParam<base::TimeDelta>
+    kUserBypassUIReloadBubbleTimeout;
+
 // Hide activity indicators if a permission is no longer used.
 COMPONENT_EXPORT(CONTENT_SETTINGS_FEATURES)
 BASE_DECLARE_FEATURE(kImprovedSemanticsActivityIndicators);

tools/metrics/actions/actions.xml

@@ -7785,6 +7785,18 @@ should be able to be added at any place in this file.
   <description>The cookie controls UI was opened.</description>
 </action>
 
+<action name="CookieControls.Bubble.ReloadingShown">
+  <owner>kmg@google.com</owner>
+  <owner>fmacintosh@google.com</owner>
+  <description>The reloading bubble is shown.</description>
+</action>
+
+<action name="CookieControls.Bubble.ReloadingTimeout">
+  <owner>kmg@google.com</owner>
+  <owner>fmacintosh@google.com</owner>
+  <description>The reloading bubble closed after a timeout.</description>
+</action>
+
 <action name="CookieControls.Bubble.SendFeedback">
   <owner>boujane@google.com</owner>
   <owner>sauski@google.com</owner>