[mparch] Support prerendering in DocumentServiceBase

This CL replaces IsServedFromBackForwardCache() with
IsPageActivation() to include the prerendering activation
in the condition that keeps DocumentServiceBase.

Bug: 1233870
Change-Id: I3eb4302b085e5a0734296cd45929d79b7122644d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3056266
Commit-Queue: Julie Kim <jkim@igalia.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#908788}

content/browser/back_forward_cache_browsertest.cc

@@ -9038,63 +9038,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
   presentation_service.OnDelegateDestroyed();
 }
 
-namespace {
-
-// Subclass of DocumentServiceBase for test.
-class EchoImpl final : public DocumentServiceBase<mojom::Echo> {
- public:
-  EchoImpl(RenderFrameHost* render_frame_host,
-           mojo::PendingReceiver<mojom::Echo> receiver,
-           bool* deleted)
-      : DocumentServiceBase(render_frame_host, std::move(receiver)),
-        deleted_(deleted) {}
-  ~EchoImpl() final { *deleted_ = true; }
-
-  // mojom::Echo implementation
-  void EchoString(const std::string& input, EchoStringCallback callback) final {
-    std::move(callback).Run(input);
-  }
-
- private:
-  bool* deleted_;
-};
-
-}  // namespace
-
-IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, DocumentServiceBase) {
-  ASSERT_TRUE(embedded_test_server()->Start());
-  GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
-  GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
-
-  // 1) Navigate to A.
-  ASSERT_TRUE(NavigateToURL(shell(), url_a));
-  RenderFrameHostImpl* rfh_a = current_frame_host();
-  RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
-
-  mojo::Remote<mojom::Echo> echo_remote;
-  bool echo_deleted = false;
-  new EchoImpl(rfh_a, echo_remote.BindNewPipeAndPassReceiver(), &echo_deleted);
-
-  // 2) Navigate to B.
-  ASSERT_TRUE(NavigateToURL(shell(), url_b));
-
-  // - Page A should be in the cache.
-  ASSERT_FALSE(delete_observer_rfh_a.deleted());
-  EXPECT_TRUE(rfh_a->IsInBackForwardCache());
-  EXPECT_FALSE(echo_deleted);
-
-  // 3) Go back.
-  web_contents()->GetController().GoBack();
-  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-  EXPECT_FALSE(echo_deleted);
-
-  // 4) Prevent caching and navigate to B.
-  DisableBFCacheForRFHForTesting(rfh_a);
-  ASSERT_TRUE(NavigateToURL(shell(), url_b));
-  delete_observer_rfh_a.WaitUntilDeleted();
-  EXPECT_TRUE(echo_deleted);
-}
-
 IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, OutstandingFetchNotCached) {
   net::test_server::ControllableHttpResponse response(embedded_test_server(),
                                                       "/fetch");

content/browser/renderer_host/document_service_base_browsertest.cc

@@ -0,0 +1,151 @@
+// Copyright 2021 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 "base/bind.h"
+#include "content/browser/renderer_host/document_service_base_echo_impl.h"
+#include "content/public/browser/document_service_base.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/common/content_features.h"
+#include "content/public/test/browser_test.h"
+#include "content/public/test/content_browser_test.h"
+#include "content/public/test/content_browser_test_utils.h"
+#include "content/public/test/prerender_test_util.h"
+#include "content/public/test/test_utils.h"
+#include "content/shell/browser/shell.h"
+#include "content/test/content_browser_test_utils_internal.h"
+#include "content/test/echo.test-mojom.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "net/dns/mock_host_resolver.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace content {
+
+class DocumentServiceBaseBrowserTest : public ContentBrowserTest {
+ public:
+  DocumentServiceBaseBrowserTest() = default;
+  ~DocumentServiceBaseBrowserTest() override = default;
+
+  void SetUpOnMainThread() override {
+    host_resolver()->AddRule("*", "127.0.0.1");
+    ASSERT_TRUE(test_server_handle_ =
+                    embedded_test_server()->StartAndReturnHandle());
+  }
+
+  WebContents* web_contents() const { return shell()->web_contents(); }
+
+ private:
+  net::test_server::EmbeddedTestServerHandle test_server_handle_;
+};
+
+class DocumentServiceBasePrerenderingBrowserTest
+    : public DocumentServiceBaseBrowserTest {
+ public:
+  DocumentServiceBasePrerenderingBrowserTest()
+      : prerender_helper_(base::BindRepeating(
+            &DocumentServiceBasePrerenderingBrowserTest::web_contents,
+            base::Unretained(this))) {}
+  ~DocumentServiceBasePrerenderingBrowserTest() override = default;
+
+  void SetUpOnMainThread() override {
+    prerender_helper_.SetUpOnMainThread(embedded_test_server());
+    DocumentServiceBaseBrowserTest::SetUpOnMainThread();
+  }
+
+  test::PrerenderTestHelper* prerender_helper() { return &prerender_helper_; }
+
+ private:
+  test::PrerenderTestHelper prerender_helper_;
+};
+
+// Tests that DocumentServiceBase is not destroyed on prerendering activation.
+IN_PROC_BROWSER_TEST_F(DocumentServiceBasePrerenderingBrowserTest,
+                       NotClosedInPrerenderingActivation) {
+  const GURL kInitialUrl = embedded_test_server()->GetURL("/empty.html");
+  const GURL kPrerenderingUrl = embedded_test_server()->GetURL("/title1.html");
+
+  // Navigate to an initial page.
+  ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));
+
+  int host_id = prerender_helper()->AddPrerender(kPrerenderingUrl);
+  RenderFrameHost* prerendered_frame_host =
+      prerender_helper()->GetPrerenderedMainFrameHost(host_id);
+  // We should disable proactive BrowsingInstance swap for the navigation below
+  // to ensure that the speculative RFH is going to use the same
+  // BrowsingInstance as the original RFH and it's not replaced on navigation.
+  DisableProactiveBrowsingInstanceSwapFor(prerendered_frame_host);
+
+  mojo::Remote<mojom::Echo> echo_remote;
+  bool echo_deleted = false;
+  new DocumentServiceBaseEchoImpl(
+      prerendered_frame_host, echo_remote.BindNewPipeAndPassReceiver(),
+      base::BindOnce([](bool* deleted) { *deleted = true; }, &echo_deleted));
+
+  // Activate the prerendered page.
+  prerender_helper()->NavigatePrimaryPage(kPrerenderingUrl);
+  // DocumentServiceBase should not be destroyed.
+  EXPECT_FALSE(echo_deleted);
+
+  ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));
+  // It should be destroyed on navigation.
+  EXPECT_TRUE(echo_deleted);
+}
+
+class DocumentServiceBaseBFCacheBrowserTest
+    : public DocumentServiceBaseBrowserTest {
+ public:
+  DocumentServiceBaseBFCacheBrowserTest() {
+    feature_list_.InitWithFeaturesAndParameters(
+        {{features::kBackForwardCache,
+          {{"TimeToLiveInBackForwardCacheInSeconds", "3600"},
+           {"enable_same_site", "true"}}}},
+        {features::kBackForwardCacheMemoryControls});
+  }
+  ~DocumentServiceBaseBFCacheBrowserTest() override = default;
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(DocumentServiceBaseBFCacheBrowserTest,
+                       DocumentServiceBase) {
+  GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
+
+  // 1) Navigate to A.
+  ASSERT_TRUE(NavigateToURL(shell(), url_a));
+  RenderFrameHost* rfh_a =
+      web_contents()->GetMainFrame();  // current_frame_host();
+  RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
+
+  mojo::Remote<mojom::Echo> echo_remote;
+  bool echo_deleted = false;
+  new DocumentServiceBaseEchoImpl(
+      rfh_a, echo_remote.BindNewPipeAndPassReceiver(),
+      base::BindOnce([](bool* deleted) { *deleted = true; }, &echo_deleted));
+
+  // 2) Navigate to B.
+  ASSERT_TRUE(NavigateToURL(shell(), url_b));
+
+  // - Page A should be in the cache.
+  ASSERT_FALSE(delete_observer_rfh_a.deleted());
+  EXPECT_EQ(rfh_a->GetLifecycleState(),
+            RenderFrameHost::LifecycleState::kInBackForwardCache);
+  EXPECT_FALSE(echo_deleted);
+
+  // 3) Go back.
+  web_contents()->GetController().GoBack();
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+  EXPECT_FALSE(echo_deleted);
+
+  // 4) Prevent caching and navigate to B.
+  DisableBFCacheForRFHForTesting(rfh_a);
+  ASSERT_TRUE(NavigateToURL(shell(), url_b));
+  delete_observer_rfh_a.WaitUntilDeleted();
+  EXPECT_TRUE(echo_deleted);
+}
+
+}  // namespace content

content/browser/renderer_host/document_service_base_echo_impl.cc

@@ -0,0 +1,25 @@
+// Copyright 2021 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/browser/renderer_host/document_service_base_echo_impl.h"
+
+namespace content {
+
+DocumentServiceBaseEchoImpl::DocumentServiceBaseEchoImpl(
+    RenderFrameHost* render_frame_host,
+    mojo::PendingReceiver<mojom::Echo> receiver,
+    base::OnceClosure destruction_cb)
+    : DocumentServiceBase(render_frame_host, std::move(receiver)),
+      destruction_cb_(std::move(destruction_cb)) {}
+
+DocumentServiceBaseEchoImpl::~DocumentServiceBaseEchoImpl() {
+  std::move(destruction_cb_).Run();
+}
+
+void DocumentServiceBaseEchoImpl::EchoString(const std::string& input,
+                                             EchoStringCallback callback) {
+  std::move(callback).Run(input);
+}
+
+}  // namespace content

content/browser/renderer_host/document_service_base_echo_impl.h

@@ -0,0 +1,35 @@
+// Copyright 2021 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_BROWSER_RENDERER_HOST_DOCUMENT_SERVICE_BASE_ECHO_IMPL_H_
+#define CONTENT_BROWSER_RENDERER_HOST_DOCUMENT_SERVICE_BASE_ECHO_IMPL_H_
+
+#include "base/bind.h"
+#include "content/public/browser/document_service_base.h"
+#include "content/test/echo.test-mojom.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+
+namespace content {
+
+class RenderFrameHost;
+
+// Subclass of DocumentServiceBase for test.
+class DocumentServiceBaseEchoImpl final
+    : public DocumentServiceBase<mojom::Echo> {
+ public:
+  DocumentServiceBaseEchoImpl(RenderFrameHost* render_frame_host,
+                              mojo::PendingReceiver<mojom::Echo> receiver,
+                              base::OnceClosure destruction_cb);
+  ~DocumentServiceBaseEchoImpl() final;
+
+  // mojom::Echo implementation
+  void EchoString(const std::string& input, EchoStringCallback callback) final;
+
+ private:
+  base::OnceClosure destruction_cb_;
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_RENDERER_HOST_DOCUMENT_SERVICE_BASE_ECHO_IMPL_H_

content/browser/renderer_host/document_service_base_unittest.cc

@@ -6,14 +6,14 @@
 
 #include "base/bind.h"
 #include "base/run_loop.h"
+#include "content/browser/renderer_host/document_service_base_echo_impl.h"
 #include "content/public/browser/back_forward_cache.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/navigation_simulator.h"
 #include "content/public/test/test_renderer_host.h"
 #include "content/test/echo.test-mojom.h"
 #include "content/test/test_render_frame_host.h"
-#include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "url/gurl.h"
 
@@ -26,25 +26,6 @@ namespace {
 const char kFooOrigin[] = "https://foo.com";
 const char kBarOrigin[] = "https://bar.com";
 
-// Subclass of DocumentServiceBase for test.
-class EchoImpl final : public DocumentServiceBase<mojom::Echo> {
- public:
-  EchoImpl(RenderFrameHost* render_frame_host,
-           mojo::PendingReceiver<mojom::Echo> receiver,
-           base::OnceClosure destruction_cb)
-      : DocumentServiceBase(render_frame_host, std::move(receiver)),
-        destruction_cb_(std::move(destruction_cb)) {}
-  ~EchoImpl() final { std::move(destruction_cb_).Run(); }
-
-  // mojom::Echo implementation
-  void EchoString(const std::string& input, EchoStringCallback callback) final {
-    std::move(callback).Run(input);
-  }
-
- private:
-  base::OnceClosure destruction_cb_;
-};
-
 // Help functions to manipulate RenderFrameHosts.
 
 // Simulates navigation and returns the final RenderFrameHost.
@@ -82,9 +63,10 @@ class DocumentServiceBaseTest : public RenderViewHostTestHarness {
 
   void CreateEchoImpl(RenderFrameHost* rfh) {
     DCHECK(!is_echo_impl_alive_);
-    new EchoImpl(rfh, echo_remote_.BindNewPipeAndPassReceiver(),
-                 base::BindOnce(&DocumentServiceBaseTest::OnEchoImplDestructed,
-                                base::Unretained(this)));
+    new DocumentServiceBaseEchoImpl(
+        rfh, echo_remote_.BindNewPipeAndPassReceiver(),
+        base::BindOnce(&DocumentServiceBaseTest::OnEchoImplDestructed,
+                       base::Unretained(this)));
     is_echo_impl_alive_ = true;
   }
 

content/browser/renderer_host/navigation_request.h

@@ -872,7 +872,7 @@ class CONTENT_EXPORT NavigationRequest
 
   // Whether this navigation is activating an existing page (e.g. served from
   // the BackForwardCache or Prerender)
-  bool IsPageActivation() const;
+  bool IsPageActivation() const override;
 
   // See comments for |prerender_navigation_entry_|.
   void SetPrerenderNavigationEntry(

content/public/browser/document_service_base.h

@@ -20,9 +20,6 @@
 
 namespace content {
 
-class NavigationHandle;
-class RenderFrameHost;
-
 // Base class for mojo interface implementations tied to a document's lifetime.
 // The service will be destroyed when any of the following happens:
 // 1. mojo interface connection error happened,
@@ -86,11 +83,16 @@ class DocumentServiceBase : public Interface, public WebContentsObserver {
 
     if (!navigation_handle->HasCommitted() ||
         navigation_handle->IsSameDocument() ||
-        navigation_handle->IsServedFromBackForwardCache()) {
+        navigation_handle->IsPageActivation()) {
       return;
     }
 
     if (navigation_handle->GetRenderFrameHost() == render_frame_host_) {
+      // DocumentServiceBase is destroyed either when RenderFrameHost is
+      // destroyed (covered by RenderFrameDeleted) or when a new document
+      // commits in the same RenderFrameHost (covered by DidFinishNavigation).
+      // Only committed non-same-document non-bfcache non-prerendering
+      // activation navigations replace a document in existing RenderFrameHost.
       DVLOG(1) << __func__ << ": Close connection on navigation.";
       Close();
     }

content/public/browser/navigation_handle.h

@@ -224,6 +224,10 @@ class CONTENT_EXPORT NavigationHandle : public base::SupportsUserData {
   // Whether the navigation is restoring a page from back-forward cache.
   virtual bool IsServedFromBackForwardCache() = 0;
 
+  // Whether this navigation is activating an existing page (e.g. served from
+  // the BackForwardCache or Prerender)
+  virtual bool IsPageActivation() const = 0;
+
   // Navigation control flow --------------------------------------------------
 
   // The net error code if an error happened prior to commit. Otherwise it will

content/public/test/mock_navigation_handle.h

@@ -68,6 +68,11 @@ class MockNavigationHandle : public NavigationHandle {
   bool IsServedFromBackForwardCache() override {
     return is_served_from_bfcache_;
   }
+  bool IsPageActivation() const override {
+    MockNavigationHandle* handle = const_cast<MockNavigationHandle*>(this);
+    return handle->IsPrerenderedPageActivation() ||
+           handle->IsServedFromBackForwardCache();
+  }
   RenderFrameHost* GetParentFrame() override {
     return render_frame_host_ ? render_frame_host_->GetParent() : nullptr;
   }