Stop using Resource::ResourceCallback and make Resource have a TaskHandle

third_party/WebKit/Source/platform/loader/fetch/Resource.cpp

 /*
     Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
     Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
     Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
     Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
     Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All
     rights reserved.
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
@@ skipping 228 matching lines @@
         WebSecurityOrigin(security_origin_),
         GetResponse().CacheStorageCacheName());
   } else {
     Platform::Current()->CacheMetadataInCacheStorage(
         GetResponse().Url(), GetResponse().ResponseTime(), nullptr, 0,
         WebSecurityOrigin(security_origin_),
         GetResponse().CacheStorageCacheName());
   }
 }
 
-// This class cannot be on-heap because the first callbackHandler() call
-// instantiates the singleton object while we can call it in the
-// pre-finalization step.
-class Resource::ResourceCallback final {
- public:
-  static ResourceCallback& CallbackHandler();
-  void Schedule(Resource*);
-  void Cancel(Resource*);
-  bool IsScheduled(Resource*) const;
-
- private:
-  ResourceCallback();
-
-  void RunTask();
-  TaskHandle task_handle_;
-  HashSet<Persistent<Resource>> resources_with_pending_clients_;
-};
-
-Resource::ResourceCallback& Resource::ResourceCallback::CallbackHandler() {
-  DEFINE_STATIC_LOCAL(ResourceCallback, callback_handler, ());
-  return callback_handler;
-}
-
-Resource::ResourceCallback::ResourceCallback() {}
-
-void Resource::ResourceCallback::Schedule(Resource* resource) {
-  if (!task_handle_.IsActive()) {
-    // WTF::unretained(this) is safe because a posted task is canceled when
-    // |task_handle_| is destroyed on the dtor of this ResourceCallback.
-    task_handle_ =
-        Platform::Current()
-            ->CurrentThread()
-            ->Scheduler()
-            ->LoadingTaskRunner()
-            ->PostCancellableTask(
-                BLINK_FROM_HERE,
-                WTF::Bind(&ResourceCallback::RunTask, WTF::Unretained(this)));
-  }
-  resources_with_pending_clients_.insert(resource);
-}
-
-void Resource::ResourceCallback::Cancel(Resource* resource) {
-  resources_with_pending_clients_.erase(resource);
-  if (task_handle_.IsActive() && resources_with_pending_clients_.IsEmpty())
-    task_handle_.Cancel();
-}
-
-bool Resource::ResourceCallback::IsScheduled(Resource* resource) const {
-  return resources_with_pending_clients_.Contains(resource);
-}
-
-void Resource::ResourceCallback::RunTask() {
-  HeapVector<Member<Resource>> resources;
-  for (const Member<Resource>& resource : resources_with_pending_clients_)
-    resources.push_back(resource.Get());
-  resources_with_pending_clients_.clear();
-
-  for (const auto& resource : resources)
-    resource->FinishPendingClients();
-}
-
 Resource::Resource(const ResourceRequest& request,
                    Type type,
                    const ResourceLoaderOptions& options)
     : load_finish_time_(0),
       identifier_(0),
       encoded_size_(0),
       encoded_size_memory_usage_(0),
       decoded_size_(0),
       overhead_size_(CalculateOverheadSize()),
       preload_count_(0),
@@ skipping 395 matching lines @@
     clients_.insert(client);
     return;
   }
 
   // If an error has occurred or we have existing data to send to the new client
   // and the resource type supprts it, send it asynchronously.
   if ((ErrorOccurred() || !GetResponse().IsNull()) &&
       !TypeNeedsSynchronousCacheHit(GetType()) &&
       !needs_synchronous_cache_hit_) {
     clients_awaiting_callback_.insert(client);
-    ResourceCallback::CallbackHandler().Schedule(this);
+    if (!async_callback_task_handle_.IsActive()) {
+      async_callback_task_handle_ =
+          Platform::Current()
+              ->CurrentThread()
+              ->Scheduler()
+              ->LoadingTaskRunner()
+              ->PostCancellableTask(BLINK_FROM_HERE,
+                                    WTF::Bind(&Resource::FinishPendingClients,
+                                              WrapWeakPersistent(this)));
+    }
     return;
   }
 
   clients_.insert(client);
   DidAddClient(client);
   return;
 }
 
 void Resource::RemoveClient(ResourceClient* client) {
   CHECK(!is_add_remove_client_prohibited_);
 
   // This code may be called in a pre-finalizer, where weak members in the
   // HashCountedSet are already swept out.
 
   if (finished_clients_.Contains(client))
     finished_clients_.erase(client);
   else if (clients_awaiting_callback_.Contains(client))
     clients_awaiting_callback_.erase(client);
   else
     clients_.erase(client);
 
   if (clients_awaiting_callback_.IsEmpty())

[yhirano, 2017/05/10 06:59:17]

The original code contains |task_runner_.IsActive()| in the condition.

... that said, it looks Cancel() does nothing when not active. Removing the
check is no problem, but I think L778-L780 should be consistent on the point.

[horo, 2017/05/10 07:05:30]

Done.

-    ResourceCallback::CallbackHandler().Cancel(this);
+    async_callback_task_handle_.Cancel();
 
   DidRemoveClientOrObserver();
 }
 
 void Resource::DidRemoveClientOrObserver() {
   if (!HasClientsOrObservers() && is_alive_) {
     is_alive_ = false;
     AllClientsAndObserversRemoved();
 
     // RFC2616 14.9.2:
@@ skipping 61 matching lines @@
 
     // When revalidation starts after waiting clients are scheduled and
     // before they are added here. In such cases, we just add the clients
     // to |clients_| without DidAddClient(), as in Resource::AddClient().
     if (!is_revalidating_)
       DidAddClient(client);
   }
 
   // It is still possible for the above loop to finish a new client
   // synchronously. If there's no client waiting we should deschedule.
-  bool scheduled = ResourceCallback::CallbackHandler().IsScheduled(this);
+  bool scheduled = async_callback_task_handle_.IsActive();
   if (scheduled && clients_awaiting_callback_.IsEmpty())
-    ResourceCallback::CallbackHandler().Cancel(this);
+    async_callback_task_handle_.Cancel();
 
   // Prevent the case when there are clients waiting but no callback scheduled.
   DCHECK(clients_awaiting_callback_.IsEmpty() || scheduled);
 }
 
 void Resource::Prune() {
   DestroyDecodedDataIfPossible();
 }
 
 void Resource::OnPurgeMemory() {
@@ skipping 277 matching lines @@
     case Resource::kMedia:
     case Resource::kManifest:
     case Resource::kMock:
       return false;
   }
   NOTREACHED();
   return false;
 }
 
 }  // namespace blink