Migrate CacheStorage to use GC mojo wrappers.

No behavior change. This CL reduces potential risks of use-after-free bugs.

Bug: 1049056
Change-Id: Ib58dec4de1c128074264e0d530071bd3a7cd5179
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2132889Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Minoru Chikamune <chikamune@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756187}

third_party/blink/renderer/modules/cache_storage/cache_storage.cc

@@ -104,7 +104,7 @@ ScriptPromise CacheStorage::open(ScriptState* script_state,
 
   // The context may be destroyed and the mojo connection unbound. However the
   // object may live on, reject any requests after the context is destroyed.
-  if (!cache_storage_remote_) {
+  if (!cache_storage_remote_.is_bound()) {
     resolver->Reject(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kInvalidStateError));
     return promise;
@@ -177,7 +177,7 @@ ScriptPromise CacheStorage::has(ScriptState* script_state,
 
   // The context may be destroyed and the mojo connection unbound. However the
   // object may live on, reject any requests after the context is destroyed.
-  if (!cache_storage_remote_) {
+  if (!cache_storage_remote_.is_bound()) {
     resolver->Reject(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kInvalidStateError));
     return promise;
@@ -237,7 +237,7 @@ ScriptPromise CacheStorage::Delete(ScriptState* script_state,
 
   // The context may be destroyed and the mojo connection unbound. However the
   // object may live on, reject any requests after the context is destroyed.
-  if (!cache_storage_remote_) {
+  if (!cache_storage_remote_.is_bound()) {
     resolver->Reject(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kInvalidStateError));
     return promise;
@@ -297,7 +297,7 @@ ScriptPromise CacheStorage::keys(ScriptState* script_state) {
 
   // The context may be destroyed and the mojo connection unbound. However the
   // object may live on, reject any requests after the context is destroyed.
-  if (!cache_storage_remote_) {
+  if (!cache_storage_remote_.is_bound()) {
     resolver->Reject(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kInvalidStateError));
     return promise;
@@ -374,7 +374,7 @@ ScriptPromise CacheStorage::MatchImpl(ScriptState* script_state,
 
   // The context may be destroyed and the mojo connection unbound. However the
   // object may live on, reject any requests after the context is destroyed.
-  if (!cache_storage_remote_) {
+  if (!cache_storage_remote_.is_bound()) {
     resolver->Reject(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kInvalidStateError));
     return promise;
@@ -457,9 +457,10 @@ ScriptPromise CacheStorage::MatchImpl(ScriptState* script_state,
 
 CacheStorage::CacheStorage(ExecutionContext* context,
                            GlobalFetch::ScopedFetcher* fetcher)
-    : ExecutionContextLifecycleObserver(context),
+    : ExecutionContextClient(context),
       scoped_fetcher_(fetcher),
       blob_client_list_(MakeGarbageCollected<CacheStorageBlobClientList>()),
+      cache_storage_remote_(context),
       ever_used_(false) {
   // See https://bit.ly/2S0zRAS for task types.
   scoped_refptr<base::SingleThreadTaskRunner> task_runner =
@@ -471,8 +472,9 @@ CacheStorage::CacheStorage(ExecutionContext* context,
     mojo::PendingRemote<mojom::blink::CacheStorage> info =
         service_worker->TakeCacheStorage();
     if (info) {
-      cache_storage_remote_ = mojo::Remote<mojom::blink::CacheStorage>(
-          std::move(info), task_runner);
+      cache_storage_remote_ =
+          HeapMojoRemote<mojom::blink::CacheStorage>(context);
+      cache_storage_remote_.Bind(std::move(info), task_runner);
       return;
     }
   }
@@ -497,8 +499,9 @@ bool CacheStorage::HasPendingActivity() const {
 void CacheStorage::Trace(Visitor* visitor) {
   visitor->Trace(scoped_fetcher_);
   visitor->Trace(blob_client_list_);
+  visitor->Trace(cache_storage_remote_);
   ScriptWrappable::Trace(visitor);
-  ExecutionContextLifecycleObserver::Trace(visitor);
+  ExecutionContextClient::Trace(visitor);
 }
 
 bool CacheStorage::IsAllowed(ScriptState* script_state) {
@@ -509,8 +512,4 @@ bool CacheStorage::IsAllowed(ScriptState* script_state) {
   return allowed_.value();
 }
 
-void CacheStorage::ContextDestroyed() {
-  cache_storage_remote_.reset();
-}
-
 }  // namespace blink

third_party/blink/renderer/modules/cache_storage/cache_storage.h

@@ -8,7 +8,6 @@
 #include <memory>
 #include "base/macros.h"
 #include "base/optional.h"
-#include "mojo/public/cpp/bindings/remote.h"
 #include "third_party/blink/public/mojom/cache_storage/cache_storage.mojom-blink-forward.h"
 #include "third_party/blink/renderer/bindings/core/v8/active_script_wrappable.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
@@ -17,6 +16,7 @@
 #include "third_party/blink/renderer/modules/cache_storage/cache.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
 #include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
+#include "third_party/blink/renderer/platform/mojo/heap_mojo_remote.h"
 #include "third_party/blink/renderer/platform/wtf/forward.h"
 #include "third_party/blink/renderer/platform/wtf/hash_map.h"
 
@@ -27,7 +27,7 @@ class MultiCacheQueryOptions;
 
 class CacheStorage final : public ScriptWrappable,
                            public ActiveScriptWrappable<CacheStorage>,
-                           public ExecutionContextLifecycleObserver {
+                           public ExecutionContextClient {
   DEFINE_WRAPPERTYPEINFO();
   USING_GARBAGE_COLLECTED_MIXIN(CacheStorage);
 
@@ -46,7 +46,6 @@ class CacheStorage final : public ScriptWrappable,
 
   bool HasPendingActivity() const override;
   void Trace(Visitor*) override;
-  void ContextDestroyed() override;
 
  private:
   ScriptPromise MatchImpl(ScriptState*,
@@ -58,7 +57,7 @@ class CacheStorage final : public ScriptWrappable,
   Member<GlobalFetch::ScopedFetcher> scoped_fetcher_;
   Member<CacheStorageBlobClientList> blob_client_list_;
 
-  mojo::Remote<mojom::blink::CacheStorage> cache_storage_remote_;
+  HeapMojoRemote<mojom::blink::CacheStorage> cache_storage_remote_;
   base::Optional<bool> allowed_;
   bool ever_used_;