Quota: Minor ownership cleanup around ClientUsageTracker.

This CL applies straightforward fixes for the following code health
issues:

1) ClientUsageTracker's constructor stores a reference to its
   SpecialStoragePolicy parameter, so the parameter should be a
   scoped_refptr, not a raw pointer.

2) SupportsWeakPtr is deprecated in favor of using WeakPtrFactory
   directly.

3) DISALLOW_COPY_AND_ASSIGN() is deprecated in favor of explicitly
   =delete-ing the copy constructor and copy assignment operators.

Change-Id: I1b624abfa4779005401623278eca6584be1dbf23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2281087
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Jarryd Goodman <jarrydg@chromium.org>
Commit-Queue: Jarryd Goodman <jarrydg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785987}

storage/browser/quota/client_usage_tracker.cc

@@ -71,13 +71,13 @@ ClientUsageTracker::ClientUsageTracker(
     UsageTracker* tracker,
     scoped_refptr<QuotaClient> client,
     blink::mojom::StorageType type,
-    SpecialStoragePolicy* special_storage_policy)
-    : client_(client),
+    scoped_refptr<SpecialStoragePolicy> special_storage_policy)
+    : client_(std::move(client)),
       type_(type),
       global_limited_usage_(0),
       global_unlimited_usage_(0),
       global_usage_retrieved_(false),
-      special_storage_policy_(special_storage_policy) {
+      special_storage_policy_(std::move(special_storage_policy)) {
   DCHECK(client_);
   if (special_storage_policy_.get())
     special_storage_policy_->AddObserver(this);
@@ -105,9 +105,10 @@ void ClientUsageTracker::GetGlobalLimitedUsage(UsageCallback callback) {
 
   AccumulateInfo* info = new AccumulateInfo;
   info->pending_jobs = non_cached_limited_origins_by_host_.size() + 1;
-  auto accumulator = base::BindRepeating(
-      &ClientUsageTracker::AccumulateLimitedOriginUsage, AsWeakPtr(),
-      base::Owned(info), AdaptCallbackForRepeating(std::move(callback)));
+  auto accumulator =
+      base::BindRepeating(&ClientUsageTracker::AccumulateLimitedOriginUsage,
+                          weak_factory_.GetWeakPtr(), base::Owned(info),
+                          AdaptCallbackForRepeating(std::move(callback)));
 
   for (const auto& host_and_origins : non_cached_limited_origins_by_host_) {
     for (const auto& origin : host_and_origins.second)
@@ -129,7 +130,7 @@ void ClientUsageTracker::GetGlobalUsage(GlobalUsageCallback callback) {
 
   client_->GetOriginsForType(
       type_, base::BindOnce(&ClientUsageTracker::DidGetOriginsForGlobalUsage,
-                            AsWeakPtr(), std::move(callback)));
+                            weak_factory_.GetWeakPtr(), std::move(callback)));
 }
 
 void ClientUsageTracker::GetHostUsage(const std::string& host,
@@ -149,7 +150,7 @@ void ClientUsageTracker::GetHostUsage(const std::string& host,
   client_->GetOriginsForHost(
       type_, host,
       base::BindOnce(&ClientUsageTracker::DidGetOriginsForHostUsage,
-                     AsWeakPtr(), host));
+                     weak_factory_.GetWeakPtr(), host));
 }
 
 void ClientUsageTracker::UpdateUsageCache(const url::Origin& origin,
@@ -284,8 +285,8 @@ void ClientUsageTracker::DidGetOriginsForGlobalUsage(
   // fire the sentinel callback at the end.
   info->pending_jobs = origins_by_host.size() + 1;
   auto accumulator = base::BindRepeating(
-      &ClientUsageTracker::AccumulateHostUsage, AsWeakPtr(), base::Owned(info),
-      base::AdaptCallbackForRepeating(std::move(callback)));
+      &ClientUsageTracker::AccumulateHostUsage, weak_factory_.GetWeakPtr(),
+      base::Owned(info), base::AdaptCallbackForRepeating(std::move(callback)));
 
   for (const auto& host_and_origins : origins_by_host) {
     const std::string& host = host_and_origins.first;
@@ -337,7 +338,7 @@ void ClientUsageTracker::GetUsageForOrigins(
   info->pending_jobs = origins.size() + 1;
   auto accumulator =
       base::BindRepeating(&ClientUsageTracker::AccumulateOriginUsage,
-                          AsWeakPtr(), base::Owned(info), host);
+                          weak_factory_.GetWeakPtr(), base::Owned(info), host);
 
   for (const auto& origin : origins) {
     DCHECK_EQ(host, origin.host());

storage/browser/quota/client_usage_tracker.h

@@ -14,7 +14,6 @@
 #include <vector>
 
 #include "base/callback.h"
-#include "base/macros.h"
 #include "base/sequence_checker.h"
 #include "storage/browser/quota/quota_callbacks.h"
 #include "storage/browser/quota/quota_client.h"
@@ -43,15 +42,19 @@ enum class InvalidOriginReason {
 // A UsageTracker object will own one ClientUsageTracker instance per client.
 // This class is not thread-safe. All methods other than the constructor must be
 // called on the same sequence.
-class ClientUsageTracker : public SpecialStoragePolicy::Observer,
-                           public base::SupportsWeakPtr<ClientUsageTracker> {
+class ClientUsageTracker : public SpecialStoragePolicy::Observer {
  public:
   using OriginSetByHost = std::map<std::string, std::set<url::Origin>>;
 
-  ClientUsageTracker(UsageTracker* tracker,
+  ClientUsageTracker(
+      UsageTracker* tracker,
       scoped_refptr<QuotaClient> client,
       blink::mojom::StorageType type,
-                     SpecialStoragePolicy* special_storage_policy);
+      scoped_refptr<SpecialStoragePolicy> special_storage_policy);
+
+  ClientUsageTracker(const ClientUsageTracker&) = delete;
+  ClientUsageTracker& operator=(const ClientUsageTracker&) = delete;
+
   ~ClientUsageTracker() override;
 
   void GetGlobalLimitedUsage(UsageCallback callback);
@@ -126,11 +129,11 @@ class ClientUsageTracker : public SpecialStoragePolicy::Observer,
       int64_t>
       host_usage_accumulators_;
 
-  scoped_refptr<SpecialStoragePolicy> special_storage_policy_;
+  const scoped_refptr<SpecialStoragePolicy> special_storage_policy_;
 
   SEQUENCE_CHECKER(sequence_checker_);
 
-  DISALLOW_COPY_AND_ASSIGN(ClientUsageTracker);
+  base::WeakPtrFactory<ClientUsageTracker> weak_factory_{this};
 };
 
 }  // namespace storage

storage/browser/quota/usage_tracker.cc

@@ -40,7 +40,7 @@ struct UsageTracker::AccumulateInfo {
 UsageTracker::UsageTracker(
     const base::flat_map<QuotaClient*, QuotaClientType>& client_types,
     blink::mojom::StorageType type,
-    SpecialStoragePolicy* special_storage_policy)
+    scoped_refptr<SpecialStoragePolicy> special_storage_policy)
     : type_(type) {
   size_t client_count = 0;
 

storage/browser/quota/usage_tracker.h

@@ -16,7 +16,7 @@
 #include "base/callback.h"
 #include "base/component_export.h"
 #include "base/containers/flat_map.h"
-#include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/sequence_checker.h"
 #include "storage/browser/quota/quota_callbacks.h"
 #include "storage/browser/quota/quota_client.h"
@@ -43,7 +43,11 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) UsageTracker
   UsageTracker(
       const base::flat_map<QuotaClient*, QuotaClientType>& client_types,
       blink::mojom::StorageType type,
-      SpecialStoragePolicy* special_storage_policy);
+      scoped_refptr<SpecialStoragePolicy> special_storage_policy);
+
+  UsageTracker(const UsageTracker&) = delete;
+  UsageTracker& operator=(const UsageTracker&) = delete;
+
   ~UsageTracker() override;
 
   blink::mojom::StorageType type() const {
@@ -102,7 +106,6 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) UsageTracker
   SEQUENCE_CHECKER(sequence_checker_);
 
   base::WeakPtrFactory<UsageTracker> weak_factory_{this};
-  DISALLOW_COPY_AND_ASSIGN(UsageTracker);
 };
 
 }  // namespace storage