Use the same SessionStorageNamespace for prerendering

Currently there is an issue that the Session Storage is not carried over to the prerendering page. This is because a new Session Storage Namespace is used for the prerendering page. To fix this issue, this CL changes PrerenderHost::PageHolder to copy the Session Storage Namespace from the initiator page to the prerendering page. We don’t want the Session Storage state in the storage service be updated by the prerendering page. And we want to synchronize the Session Storage state of the prerendering page with the initiator page when the prerendering page is activated. So this CL introduces a flag |is_session_storage_for_prerendering_| in CachedStorageArea, and make CachedStorageArea not to send the changes of the Session Storage state to the storage service, and make StorageArea recreate |cached_area_| when the prerendering page is activated. This is the "clone & swap" mechanism for session storage in prerendering described in whatwg/storage#119. This CL still has an issue that when the initial renderer process is reused after the back navigation from a prerendered page, the Session Storage state is not correctly propagated to the initial renderer process. This issue will be fixed in the next CL. https://crrev.com/c/2849654 Bug: 1197383 Change-Id: Ib43386ccf75f8c867bcddb4b77b333ee0d5b5d79 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2850242 Reviewed-by: Kinuko Yasuda <[email protected]> Reviewed-by: Matt Falkenhagen <[email protected]> Reviewed-by: Marijn Kruisselbrink <[email protected]> Commit-Queue: Tsuyoshi Horo <[email protected]> Cr-Commit-Position: refs/heads/master@{#881985} NOKEYCHECK=True GitOrigin-RevId: eefb8c561ab863863b0541125df363fef040eabb
mjfroman · May 12, 2021 · 7df9153 · 7df9153
1 parent 3e7ca20
commit 7df9153
Show file tree

Hide file tree

Showing 16 changed files with 375 additions and 17 deletions.
diff --git a/blink/renderer/core/dom/document.cc b/blink/renderer/core/dom/document.cc
@@ -8079,6 +8079,12 @@ const Node* Document::GetFindInPageActiveMatchNode() const {
   return find_in_page_active_match_node_;
 }
 
+void Document::RegisterWillDispatchPrerenderchangeCallback(
+    base::OnceClosure closure) {
+  DCHECK(is_prerendering_);
+  will_dispatch_prerenderchange_callbacks_.push_back(std::move(closure));
+}
+
 void Document::ActivateForPrerendering() {
   DCHECK(RuntimeEnabledFeatures::Prerender2Enabled());
 
@@ -8097,6 +8103,12 @@ void Document::ActivateForPrerendering() {
   if (DocumentLoader* loader = Loader())
     loader->NotifyPrerenderingDocumentActivated();
 
+  Vector<base::OnceClosure> callbacks;
+  callbacks.swap(will_dispatch_prerenderchange_callbacks_);
+  for (auto& callback : callbacks) {
+    std::move(callback).Run();
+  }
+
   DispatchEvent(*Event::Create(event_type_names::kPrerenderingchange));
 
   if (LocalFrame* frame = GetFrame())

diff --git a/blink/renderer/core/dom/document.h b/blink/renderer/core/dom/document.h
@@ -323,6 +323,8 @@ class CORE_EXPORT Document : public ContainerNode,
 
   bool IsPrerendering() const { return is_prerendering_; }
 
+  void RegisterWillDispatchPrerenderchangeCallback(base::OnceClosure);
+
   network::mojom::ReferrerPolicy GetReferrerPolicy() const;
 
   bool DocumentPolicyFeatureObserved(
@@ -1838,6 +1840,10 @@ class CORE_EXPORT Document : public ContainerNode,
   // https://github.com/jeremyroman/alternate-loading-modes/blob/main/prerendering-state.md#documentprerendering
   bool is_prerendering_;
 
+  // Callbacks to execute upon activation of a prerendered page, just before the
+  // prerenderchange event is dispatched.
+  Vector<base::OnceClosure> will_dispatch_prerenderchange_callbacks_;
+
   bool evaluate_media_queries_on_style_recalc_;
 
   // If we do ignore the pending stylesheet count, then we need to add a boolean

diff --git a/blink/renderer/modules/storage/cached_storage_area.cc b/blink/renderer/modules/storage/cached_storage_area.cc
@@ -102,10 +102,13 @@ bool CachedStorageArea::SetItem(const String& key,
   KURL page_url = source->GetPageUrl();
   String source_id = areas_->at(source);
   String source_string = PackSource(page_url, source_id);
-  remote_area_->Put(StringToUint8Vector(key, GetKeyFormat()),
-                    StringToUint8Vector(value, value_format),
-                    optional_old_value, source_string,
-                    MakeSuccessCallback(source));
+
+  if (!is_session_storage_for_prerendering_) {
+    remote_area_->Put(StringToUint8Vector(key, GetKeyFormat()),
+                      StringToUint8Vector(value, value_format),
+                      optional_old_value, source_string,
+                      MakeSuccessCallback(source));
+  }
   if (!IsSessionStorage())
     EnqueuePendingMutation(key, value, old_value, source_string);
   else if (old_value != value)
@@ -127,9 +130,11 @@ void CachedStorageArea::RemoveItem(const String& key, Source* source) {
   KURL page_url = source->GetPageUrl();
   String source_id = areas_->at(source);
   String source_string = PackSource(page_url, source_id);
-  remote_area_->Delete(StringToUint8Vector(key, GetKeyFormat()),
-                       optional_old_value, source_string,
-                       MakeSuccessCallback(source));
+  if (!is_session_storage_for_prerendering_) {
+    remote_area_->Delete(StringToUint8Vector(key, GetKeyFormat()),
+                         optional_old_value, source_string,
+                         MakeSuccessCallback(source));
+  }
   if (!IsSessionStorage())
     EnqueuePendingMutation(key, String(), old_value, source_string);
   else
@@ -164,8 +169,10 @@ void CachedStorageArea::Clear(Source* source) {
   KURL page_url = source->GetPageUrl();
   String source_id = areas_->at(source);
   String source_string = PackSource(page_url, source_id);
-  remote_area_->DeleteAll(source_string, std::move(new_observer),
-                          MakeSuccessCallback(source));
+  if (!is_session_storage_for_prerendering_) {
+    remote_area_->DeleteAll(source_string, std::move(new_observer),
+                            MakeSuccessCallback(source));
+  }
   if (!IsSessionStorage())
     EnqueuePendingMutation(String(), String(), String(), source_string);
   else if (!already_empty)
@@ -182,10 +189,12 @@ CachedStorageArea::CachedStorageArea(
     AreaType type,
     scoped_refptr<const SecurityOrigin> origin,
     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-    StorageNamespace* storage_namespace)
+    StorageNamespace* storage_namespace,
+    bool is_session_storage_for_prerendering)
     : type_(type),
       origin_(std::move(origin)),
       storage_namespace_(storage_namespace),
+      is_session_storage_for_prerendering_(is_session_storage_for_prerendering),
       task_runner_(std::move(task_runner)),
       areas_(MakeGarbageCollected<HeapHashMap<WeakMember<Source>, String>>()) {
   BindStorageArea();
@@ -218,6 +227,7 @@ void CachedStorageArea::BindStorageArea(
 
 void CachedStorageArea::ResetConnection(
     mojo::PendingRemote<mojom::blink::StorageArea> new_area) {
+  DCHECK(!is_session_storage_for_prerendering_);
   remote_area_.reset();
   BindStorageArea(std::move(new_area));
 

diff --git a/blink/renderer/modules/storage/cached_storage_area.h b/blink/renderer/modules/storage/cached_storage_area.h
@@ -63,7 +63,8 @@ class MODULES_EXPORT CachedStorageArea
   CachedStorageArea(AreaType type,
                     scoped_refptr<const SecurityOrigin> origin,
                     scoped_refptr<base::SingleThreadTaskRunner> ipc_runner,
-                    StorageNamespace* storage_namespace);
+                    StorageNamespace* storage_namespace,
+                    bool is_session_storage_for_prerendering);
 
   // These correspond to blink::Storage.
   unsigned GetLength();
@@ -97,6 +98,10 @@ class MODULES_EXPORT CachedStorageArea
   void ResetConnection(
       mojo::PendingRemote<mojom::blink::StorageArea> new_area = {});
 
+  bool is_session_storage_for_prerendering() const {
+    return is_session_storage_for_prerendering_;
+  }
+
   void SetRemoteAreaForTesting(
       mojo::PendingRemote<mojom::blink::StorageArea> area) {
     remote_area_.Bind(std::move(area));
@@ -177,6 +182,12 @@ class MODULES_EXPORT CachedStorageArea
   const AreaType type_;
   const scoped_refptr<const SecurityOrigin> origin_;
   const WeakPersistent<StorageNamespace> storage_namespace_;
+  // Session storage state for prerendering is initialized by cloning the
+  // primary session storage state. It is used locally by the prerendering
+  // context, and does not get propagated back to the primary state (i.e., via
+  // remote_area_). For more details:
+  // https://docs.google.com/document/d/1I5Hr8I20-C1GBr4tAXdm0U8a1RDUKHt4n7WcH4fxiSE/edit?usp=sharing
+  const bool is_session_storage_for_prerendering_;
   const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 
   std::unique_ptr<StorageAreaMap> map_;

diff --git a/blink/renderer/modules/storage/cached_storage_area_test.cc b/blink/renderer/modules/storage/cached_storage_area_test.cc
@@ -38,7 +38,7 @@ class CachedStorageAreaTest : public testing::Test {
                            : CachedStorageArea::AreaType::kLocalStorage;
     cached_area_ = base::MakeRefCounted<CachedStorageArea>(
         area_type, kOrigin, scheduler::GetSingleThreadTaskRunnerForTesting(),
-        nullptr);
+        nullptr, /*is_session_storage_for_prerendering=*/false);
     cached_area_->SetRemoteAreaForTesting(
         mock_storage_area_.GetInterfaceRemote());
     source_area_ = MakeGarbageCollected<FakeAreaSource>(kPageUrl);

diff --git a/blink/renderer/modules/storage/dom_window_storage.cc b/blink/renderer/modules/storage/dom_window_storage.cc
@@ -90,10 +90,16 @@ StorageArea* DOMWindowStorage::sessionStorage(
       StorageNamespace::From(window->GetFrame()->GetPage());
   if (!storage_namespace)
     return nullptr;
-  auto storage_area =
-      storage_namespace->GetCachedArea(window->GetSecurityOrigin());
+  scoped_refptr<CachedStorageArea> cached_storage_area;
+  if (window->document()->IsPrerendering()) {
+    cached_storage_area = storage_namespace->CreateCachedAreaForPrerender(
+        window->GetSecurityOrigin());
+  } else {
+    cached_storage_area =
+        storage_namespace->GetCachedArea(window->GetSecurityOrigin());
+  }
   session_storage_ =
-      StorageArea::Create(window, std::move(storage_area),
+      StorageArea::Create(window, std::move(cached_storage_area),
                           StorageArea::StorageType::kSessionStorage);
 
   if (!session_storage_->CanAccessStorage()) {

diff --git a/blink/renderer/modules/storage/storage_area.cc b/blink/renderer/modules/storage/storage_area.cc
@@ -41,6 +41,7 @@
 #include "third_party/blink/renderer/modules/storage/storage_namespace.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/weborigin/security_origin.h"
+#include "third_party/blink/renderer/platform/wtf/functional.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
@@ -73,6 +74,11 @@ StorageArea::StorageArea(LocalDOMWindow* window,
   DCHECK(window);
   DCHECK(cached_area_);
   cached_area_->RegisterSource(this);
+  if (cached_area_->is_session_storage_for_prerendering()) {
+    DomWindow()->document()->RegisterWillDispatchPrerenderchangeCallback(
+        WTF::Bind(&StorageArea::OnDocumentActivatedForPrerendering,
+                  WrapWeakPersistent(this)));
+  }
 }
 
 unsigned StorageArea::length(ExceptionState& exception_state) const {
@@ -246,4 +252,18 @@ blink::WebScopedVirtualTimePauser StorageArea::CreateWebScopedVirtualTimePauser(
       ->CreateWebScopedVirtualTimePauser(name, duration);
 }
 
+void StorageArea::OnDocumentActivatedForPrerendering() {
+  StorageNamespace* storage_namespace =
+      StorageNamespace::From(DomWindow()->GetFrame()->GetPage());
+  if (!storage_namespace)
+    return;
+
+  // Swap out the session storage state used within prerendering, and replace it
+  // with the normal session storage state. For more details:
+  // https://docs.google.com/document/d/1I5Hr8I20-C1GBr4tAXdm0U8a1RDUKHt4n7WcH4fxiSE/edit?usp=sharing
+  cached_area_ =
+      storage_namespace->GetCachedArea(DomWindow()->GetSecurityOrigin());
+  cached_area_->RegisterSource(this);
+}
+
 }  // namespace blink
diff --git a/blink/renderer/modules/storage/storage_area.h b/blink/renderer/modules/storage/storage_area.h
@@ -92,7 +92,10 @@ class StorageArea final : public ScriptWrappable,
 
  private:
   void RecordModificationInMetrics();
-  const scoped_refptr<CachedStorageArea> cached_area_;
+
+  void OnDocumentActivatedForPrerendering();
+
+  scoped_refptr<CachedStorageArea> cached_area_;
   StorageType storage_type_;
   const bool should_enqueue_events_;
 

diff --git a/blink/renderer/modules/storage/storage_namespace.cc b/blink/renderer/modules/storage/storage_namespace.cc
@@ -100,11 +100,23 @@ scoped_refptr<CachedStorageArea> StorageNamespace::GetCachedArea(
   result = base::MakeRefCounted<CachedStorageArea>(
       IsSessionStorage() ? CachedStorageArea::AreaType::kSessionStorage
                          : CachedStorageArea::AreaType::kLocalStorage,
-      origin, controller_->TaskRunner(), this);
+      origin, controller_->TaskRunner(), this,
+      /*is_session_storage_for_prerendering=*/false);
   cached_areas_.insert(std::move(origin), result);
   return result;
 }
 
+scoped_refptr<CachedStorageArea> StorageNamespace::CreateCachedAreaForPrerender(
+    const SecurityOrigin* origin_ptr) {
+  DCHECK((IsSessionStorage()));
+  scoped_refptr<const SecurityOrigin> origin(origin_ptr);
+  return base::MakeRefCounted<CachedStorageArea>(
+      IsSessionStorage() ? CachedStorageArea::AreaType::kSessionStorage
+                         : CachedStorageArea::AreaType::kLocalStorage,
+      origin, controller_->TaskRunner(), this,
+      /*is_session_storage_for_prerendering=*/true);
+}
+
 void StorageNamespace::CloneTo(const String& target) {
   DCHECK(IsSessionStorage()) << "Cannot clone a local storage namespace.";
   EnsureConnected();

diff --git a/blink/renderer/modules/storage/storage_namespace.h b/blink/renderer/modules/storage/storage_namespace.h
@@ -84,6 +84,9 @@ class MODULES_EXPORT StorageNamespace final
 
   scoped_refptr<CachedStorageArea> GetCachedArea(const SecurityOrigin* origin);
 
+  scoped_refptr<CachedStorageArea> CreateCachedAreaForPrerender(
+      const SecurityOrigin* origin);
+
   // Only valid to call this if |this| and |target| are session storage
   // namespaces.
   void CloneTo(const String& target);

diff --git a/..._tests/wpt_internal/prerender/resources/session-storage-carry-over-to-prerender-page.html b/..._tests/wpt_internal/prerender/resources/session-storage-carry-over-to-prerender-page.html
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="utils.js"></script>
+<script src="session-storage-utils.js"></script>
+<script>
+RunSessionStorageTest(async (isPrerendering, url, prerenderChannel, done) => {
+  if (!isPrerendering) {
+    sessionStorage.setItem('set by initiator page', '1');
+    startPrerendering(url);
+  } else {
+    assert_equals(
+        getSessionStorageKeys(),
+        'set by initiator page',
+        'The session storage item set by the initiator page must be carried' +
+        ' over to the prerendering page.');
+    done();
+  }
+});
+</script>
diff --git a/...b_tests/wpt_internal/prerender/resources/session-storage-isolated-while-prerendering.html b/...b_tests/wpt_internal/prerender/resources/session-storage-isolated-while-prerendering.html
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="utils.js"></script>
+<script src="session-storage-utils.js"></script>
+<script>
+RunSessionStorageTest(async (isPrerendering, url, prerenderChannel, done) => {
+  if (!isPrerendering) {
+    startPrerendering(url);
+    // Wait for the message from the prerendering page.
+    assert_equals(
+        await getNextMessage(prerenderChannel),
+        'From prerendering page 1')
+
+    // Add an item to the session storage.
+    sessionStorage.setItem('set by initiator page', '1');
+
+    // Send the message to the prerendering page.
+    prerenderChannel.postMessage('From initiator page');
+
+  } else {
+    sessionStorage.setItem('set by prerendering page', '1');
+
+    // Send the message to the initiator page.
+    prerenderChannel.postMessage('From prerendering page 1');
+
+    // Wait for the message from the initiator page.
+    assert_equals(
+        await getNextMessage(prerenderChannel),
+        'From initiator page');
+
+    assert_equals(
+        getSessionStorageKeys(),
+        'set by prerendering page',
+        'The session storage item added by the initiator page after the ' +
+        'prerendering page accessed the session storage must not be visible ' +
+        'in the prerendering page.');
+    done();
+  }
+});
+</script>
diff --git a/...web_tests/wpt_internal/prerender/resources/session-storage-no-leak-to-initiator-page.html b/...web_tests/wpt_internal/prerender/resources/session-storage-no-leak-to-initiator-page.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="utils.js"></script>
+<script src="session-storage-utils.js"></script>
+<script>
+RunSessionStorageTest(async (isPrerendering, url, prerenderChannel, done) => {
+  if (!isPrerendering) {
+    startPrerendering(url);
+
+    // Wait for the message from the prerendering page.
+    assert_equals(
+        await getNextMessage(prerenderChannel),
+        'From prerendering page')
+
+    assert_equals(
+        getSessionStorageKeys(),
+        '',
+        'The session storage item set by the prerendering page must not be ' +
+        'visible in the initiator page.');
+
+    done();
+  } else {
+    sessionStorage.setItem('set by prerendering page', '1');
+
+    assert_equals(
+        getSessionStorageKeys(),
+        'set by prerendering page',
+        'The session storage item must have been added by the prerendering' +
+        ' page.');
+    // Send the message to the initiator page.
+    prerenderChannel.postMessage('From prerendering page');
+  }
+});
+</script>