[bfcache] Add a way to test bfcache disabling.

Add BackForwardCacheTestHelper class, which can check
whether a particular frame was disabled for a given
reason.

R=clamy@chromium.org,arthursonzogni@chromium.org
TBR=crharrison@chromium.org
CC=​​lowell@chromium.org,hajimehoshi@chromium.org,sreejakshetty@chromium.org,carlscab@google.com
BUG=1001087

Change-Id: I4e5b3649e412e3e4a6c97b572597958b02f1382e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1833603
Commit-Queue: Alexander Timin <altimin@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703278}

chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc

@@ -58,11 +58,13 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/url_constants.h"
+#include "content/public/test/back_forward_cache_util.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
@@ -797,4 +799,24 @@ IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest, OpenInNewWindow) {
                kDontCheckTitle);
 }
 
+IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest, PopupsDisableBackForwardCache) {
+  content::BackForwardCacheDisabledTester tester;
+
+  const GURL url1(
+      embedded_test_server()->GetURL("/popup_blocker/popup-many.html"));
+  content::RenderFrameHost* main_frame =
+      browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame();
+  int process_id = main_frame->GetProcess()->GetID();
+  int frame_routing_id = main_frame->GetRoutingID();
+  const GURL url2(embedded_test_server()->GetURL("/title1.html"));
+
+  // Navigate to the page
+  ui_test_utils::NavigateToURL(browser(), url1);
+  // Navigate away while having blocked popups. This should block bfcache.
+  ui_test_utils::NavigateToURL(browser(), url2);
+
+  EXPECT_TRUE(tester.IsDisabledForFrameWithReason(process_id, frame_routing_id,
+                                                  "PopupBlockerTabHelper"));
+}
+
 }  // namespace

chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc

@@ -19,6 +19,7 @@
 #include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/common/chrome_render_frame.mojom.h"
 #include "chrome/common/render_messages.h"
+#include "content/public/browser/back_forward_cache.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/page_navigator.h"
@@ -70,6 +71,16 @@ void PopupBlockerTabHelper::DidFinishNavigation(
   if (!blocked_popups_.empty()) {
     blocked_popups_.clear();
     HidePopupNotification();
+
+    // With back-forward cache we can restore the page, but |blocked_popups_|
+    // are lost here and can't be restored at the moment.
+    // Disable bfcache here to avoid potential loss of the page state.
+    web_contents()
+        ->GetController()
+        .GetBackForwardCache()
+        .DisableForRenderFrameHost(
+            navigation_handle->GetPreviousRenderFrameHostId(),
+            "PopupBlockerTabHelper");
   }
 }
 

content/browser/frame_host/back_forward_cache_impl.cc

@@ -143,11 +143,14 @@ std::map<std::string, std::vector<std::string>> SetAllowedURLs() {
   return allowed_urls;
 }
 
+BackForwardCacheTestDelegate* g_bfcache_disabled_test_observer = nullptr;
+
 }  // namespace
 
 BackForwardCacheImpl::Entry::Entry(std::unique_ptr<RenderFrameHostImpl> rfh,
                                    RenderFrameProxyHostMap proxies)
     : render_frame_host(std::move(rfh)), proxy_hosts(std::move(proxies)) {}
+
 BackForwardCacheImpl::Entry::~Entry() {}
 
 std::string BackForwardCacheImpl::CanStoreDocumentResult::ToString() {
@@ -214,6 +217,16 @@ BackForwardCacheImpl::CanStoreDocumentResult::CanStoreDocumentResult(
       reason(reason),
       blocklisted_features(blocklisted_features) {}
 
+BackForwardCacheTestDelegate::BackForwardCacheTestDelegate() {
+  DCHECK(!g_bfcache_disabled_test_observer);
+  g_bfcache_disabled_test_observer = this;
+}
+
+BackForwardCacheTestDelegate::~BackForwardCacheTestDelegate() {
+  DCHECK_EQ(g_bfcache_disabled_test_observer, this);
+  g_bfcache_disabled_test_observer = nullptr;
+}
+
 BackForwardCacheImpl::BackForwardCacheImpl()
     : allowed_urls_(SetAllowedURLs()), weak_factory_(this) {}
 BackForwardCacheImpl::~BackForwardCacheImpl() = default;
@@ -396,6 +409,9 @@ void BackForwardCacheImpl::PostTaskToDestroyEvictedFrames() {
 void BackForwardCacheImpl::DisableForRenderFrameHost(GlobalFrameRoutingId id,
                                                      base::StringPiece reason) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (g_bfcache_disabled_test_observer)
+    g_bfcache_disabled_test_observer->OnDisabledForFrameWithReason(id, reason);
+
   auto* rfh = RenderFrameHostImpl::FromID(id);
   if (rfh)
     rfh->DisallowBackForwardCache();

content/browser/frame_host/back_forward_cache_impl.h

@@ -198,6 +198,19 @@ class CONTENT_EXPORT BackForwardCacheImpl : public BackForwardCache {
   DISALLOW_COPY_AND_ASSIGN(BackForwardCacheImpl);
 };
 
+// Allow external code to be notified when back-forward cache is disabled for a
+// RenderFrameHost. This should be used only by the testing infrastructure which
+// want to know the exact reason why the cache was disabled. There can be only
+// one observer.
+class CONTENT_EXPORT BackForwardCacheTestDelegate {
+ public:
+  BackForwardCacheTestDelegate();
+  virtual ~BackForwardCacheTestDelegate();
+
+  virtual void OnDisabledForFrameWithReason(GlobalFrameRoutingId id,
+                                            base::StringPiece reason) = 0;
+};
+
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_BACK_FORWARD_CACHE_IMPL_H_

content/public/test/back_forward_cache_util.cc

+// Copyright 2019 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 "content/public/test/back_forward_cache_util.h"
+
+#include <map>
+#include <set>
+
+#include "content/browser/frame_host/back_forward_cache_impl.h"
+#include "content/browser/frame_host/render_frame_host_impl.h"
+#include "content/public/browser/global_routing_id.h"
+
+namespace content {
+
+class BackForwardCacheDisabledTester::Impl
+    : public BackForwardCacheTestDelegate {
+ public:
+  bool IsDisabledForFrameWithReason(GlobalFrameRoutingId id,
+                                    base::StringPiece reason) {
+    return disable_reasons_[id].count(std::string(reason)) != 0;
+  }
+
+  void OnDisabledForFrameWithReason(GlobalFrameRoutingId id,
+                                    base::StringPiece reason) override {
+    disable_reasons_[id].insert(std::string(reason));
+  }
+
+ private:
+  std::map<GlobalFrameRoutingId, std::set<std::string>> disable_reasons_;
+};
+
+BackForwardCacheDisabledTester::BackForwardCacheDisabledTester()
+    : impl_(std::make_unique<Impl>()) {}
+
+BackForwardCacheDisabledTester::~BackForwardCacheDisabledTester() {}
+
+bool BackForwardCacheDisabledTester::IsDisabledForFrameWithReason(
+    int process_id,
+    int frame_routing_id,
+    base::StringPiece reason) {
+  return impl_->IsDisabledForFrameWithReason(
+      GlobalFrameRoutingId{process_id, frame_routing_id}, reason);
+}
+
+}  // namespace content

content/public/test/back_forward_cache_util.h

+// Copyright 2019 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.
+
+#ifndef CONTENT_PUBLIC_TEST_BACK_FORWARD_CACHE_UTIL_H_
+#define CONTENT_PUBLIC_TEST_BACK_FORWARD_CACHE_UTIL_H_
+
+#include "base/strings/string_piece.h"
+
+namespace content {
+class BackForwardCacheImpl;
+
+// This is a helper class to check in the tests that back-forward cache
+// was disabled for a particular reason.
+//
+// This class should be created in the beginning of the test and will
+// know about all BackForwardCache::DisableForRenderFrameHost which
+// happened during its lifetime.
+//
+// Typical usage pattern:
+//
+// BackForwardCacheDisabledTester helper;
+// NavigateToURL(page_with_feature);
+// NavigateToURL(away);
+// EXPECT_TRUE/FALSE(helper.IsDisabledForFrameWithReason());
+
+class BackForwardCacheDisabledTester {
+ public:
+  BackForwardCacheDisabledTester();
+  ~BackForwardCacheDisabledTester();
+
+  bool IsDisabledForFrameWithReason(int process_id,
+                                    int frame_routing_id,
+                                    base::StringPiece reason);
+
+ private:
+  // Impl has to inherit from BackForwardCacheImpl, which is
+  // a content/-internal concept, so we can include it only from
+  // .cc file.
+  class Impl;
+  std::unique_ptr<Impl> impl_;
+};
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_TEST_BACK_FORWARD_CACHE_UTIL_H_

content/test/BUILD.gn

@@ -95,6 +95,8 @@ jumbo_static_library("test_support") {
     "../public/test/accessibility_notification_waiter.h",
     "../public/test/audio_service_test_helper.cc",
     "../public/test/audio_service_test_helper.h",
+    "../public/test/back_forward_cache_util.cc",
+    "../public/test/back_forward_cache_util.h",
     "../public/test/background_sync_test_util.cc",
     "../public/test/background_sync_test_util.h",
     "../public/test/blink_test_environment.cc",