AppCache: Remove unnecessary StorageType handling in QuotaClient.

QuotaClient's methods will only be called with a StorageType in the list
of types provided to QuotaManager::RegisterClient(). AppCache only
provides kTemporary, like most storage APIs. Therefore, its QuotaClient
implementation does not need to handle other StorageType values.

Also replaced reqeusts -> requests, per Tricium's suggestion.

Bug: 1016065
Change-Id: Ib302b069e6b00c0746de98777b6bc36cadbbf061
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2249461Reviewed-by: Jarryd Goodman <jarrydg@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779454}

content/browser/appcache/appcache_quota_client.cc

@@ -16,6 +16,7 @@
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "storage/browser/quota/quota_client_type.h"
+#include "third_party/blink/public/mojom/quota/quota_types.mojom-shared.h"
 #include "third_party/blink/public/mojom/quota/quota_types.mojom.h"
 
 using blink::mojom::StorageType;
@@ -70,6 +71,7 @@ void AppCacheQuotaClient::GetOriginUsage(const url::Origin& origin,
                                          StorageType type,
                                          GetUsageCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK_EQ(type, blink::mojom::StorageType::kTemporary);
   DCHECK(!callback.is_null());
 
   if (service_is_destroyed_) {
@@ -84,11 +86,6 @@ void AppCacheQuotaClient::GetOriginUsage(const url::Origin& origin,
     return;
   }
 
-  if (type != StorageType::kTemporary) {
-    std::move(callback).Run(0);
-    return;
-  }
-
   GetUIThreadTaskRunner({})->PostTaskAndReplyWithResult(
       FROM_HERE,
       base::BindOnce(
@@ -112,25 +109,32 @@ void AppCacheQuotaClient::GetOriginUsage(const url::Origin& origin,
 void AppCacheQuotaClient::GetOriginsForType(StorageType type,
                                             GetOriginsCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  GetOriginsHelper(type, std::string(), std::move(callback));
+  DCHECK_EQ(type, blink::mojom::StorageType::kTemporary);
+  DCHECK(!callback.is_null());
+
+  GetOriginsHelper(std::string(), std::move(callback));
 }
 
 void AppCacheQuotaClient::GetOriginsForHost(StorageType type,
                                             const std::string& host,
                                             GetOriginsCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK_EQ(type, blink::mojom::StorageType::kTemporary);
   DCHECK(!callback.is_null());
+
   if (host.empty()) {
     std::move(callback).Run(std::set<url::Origin>());
     return;
   }
-  GetOriginsHelper(type, host, std::move(callback));
+  GetOriginsHelper(host, std::move(callback));
 }
 
 void AppCacheQuotaClient::DeleteOriginData(const url::Origin& origin,
                                            StorageType type,
                                            DeletionCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK_EQ(type, blink::mojom::StorageType::kTemporary);
+  DCHECK(!callback.is_null());
 
   if (service_is_destroyed_) {
     std::move(callback).Run(blink::mojom::QuotaStatusCode::kErrorAbort);
@@ -160,6 +164,10 @@ void AppCacheQuotaClient::DeleteOriginData(const url::Origin& origin,
 
 void AppCacheQuotaClient::PerformStorageCleanup(blink::mojom::StorageType type,
                                                 base::OnceClosure callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK_EQ(type, blink::mojom::StorageType::kTemporary);
+  DCHECK(!callback.is_null());
+
   std::move(callback).Run();
 }
 
@@ -174,8 +182,7 @@ void AppCacheQuotaClient::DidDeleteAppCachesForOrigin(int rv) {
   RunFront(&pending_serial_requests_);
 }
 
-void AppCacheQuotaClient::GetOriginsHelper(StorageType type,
-                                           const std::string& opt_host,
+void AppCacheQuotaClient::GetOriginsHelper(const std::string& opt_host,
                                            GetOriginsCallback callback) {
   DCHECK(!callback.is_null());
 
@@ -185,14 +192,9 @@ void AppCacheQuotaClient::GetOriginsHelper(StorageType type,
   }
 
   if (!appcache_is_ready_) {
-    pending_batch_requests_.push_back(base::BindOnce(
-        &AppCacheQuotaClient::GetOriginsHelper, base::RetainedRef(this), type,
-        opt_host, std::move(callback)));
-    return;
-  }
-
-  if (type != StorageType::kTemporary) {
-    std::move(callback).Run(std::set<url::Origin>());
+    pending_batch_requests_.push_back(
+        base::BindOnce(&AppCacheQuotaClient::GetOriginsHelper,
+                       base::RetainedRef(this), opt_host, std::move(callback)));
     return;
   }
 

content/browser/appcache/appcache_quota_client.h

@@ -63,8 +63,7 @@ class AppCacheQuotaClient : public storage::QuotaClient {
   ~AppCacheQuotaClient() override;
 
   void DidDeleteAppCachesForOrigin(int rv);
-  void GetOriginsHelper(blink::mojom::StorageType type,
-                        const std::string& opt_host,
+  void GetOriginsHelper(const std::string& opt_host,
                         GetOriginsCallback callback);
   void ProcessPendingRequests();
   void DeletePendingRequests();
@@ -75,7 +74,7 @@ class AppCacheQuotaClient : public storage::QuotaClient {
   CONTENT_EXPORT void NotifyAppCacheDestroyed();
 
   // Prior to appcache service being ready, we have to queue
-  // up reqeusts and defer acting on them until we're ready.
+  // up requests and defer acting on them until we're ready.
   RequestQueue pending_batch_requests_;
   RequestQueue pending_serial_requests_;
 

content/browser/appcache/appcache_quota_client_unittest.cc

@@ -21,7 +21,6 @@ using blink::mojom::StorageType;
 
 // Declared to shorten the line lengths.
 static const StorageType kTemp = StorageType::kTemporary;
-static const StorageType kPerm = StorageType::kPersistent;
 
 // Base class for our test fixtures.
 class AppCacheQuotaClientTest : public testing::Test {
@@ -192,15 +191,10 @@ TEST_F(AppCacheQuotaClientTest, EmptyService) {
   Call_NotifyAppCacheReady(client);
 
   EXPECT_EQ(0, GetOriginUsage(client, kOriginA, kTemp));
-  EXPECT_EQ(0, GetOriginUsage(client, kOriginA, kPerm));
   EXPECT_TRUE(GetOriginsForType(client, kTemp).empty());
-  EXPECT_TRUE(GetOriginsForType(client, kPerm).empty());
   EXPECT_TRUE(GetOriginsForHost(client, kTemp, kOriginA.host()).empty());
-  EXPECT_TRUE(GetOriginsForHost(client, kPerm, kOriginA.host()).empty());
   EXPECT_EQ(blink::mojom::QuotaStatusCode::kOk,
             DeleteOriginData(client, kTemp, kOriginA));
-  EXPECT_EQ(blink::mojom::QuotaStatusCode::kOk,
-            DeleteOriginData(client, kPerm, kOriginA));
 
   Call_NotifyAppCacheDestroyed(client);
   Call_OnQuotaManagerDestroyed(client);
@@ -212,15 +206,10 @@ TEST_F(AppCacheQuotaClientTest, NoService) {
   Call_NotifyAppCacheDestroyed(client);
 
   EXPECT_EQ(0, GetOriginUsage(client, kOriginA, kTemp));
-  EXPECT_EQ(0, GetOriginUsage(client, kOriginA, kPerm));
   EXPECT_TRUE(GetOriginsForType(client, kTemp).empty());
-  EXPECT_TRUE(GetOriginsForType(client, kPerm).empty());
   EXPECT_TRUE(GetOriginsForHost(client, kTemp, kOriginA.host()).empty());
-  EXPECT_TRUE(GetOriginsForHost(client, kPerm, kOriginA.host()).empty());
   EXPECT_EQ(blink::mojom::QuotaStatusCode::kErrorAbort,
             DeleteOriginData(client, kTemp, kOriginA));
-  EXPECT_EQ(blink::mojom::QuotaStatusCode::kErrorAbort,
-            DeleteOriginData(client, kPerm, kOriginA));
 
   Call_OnQuotaManagerDestroyed(client);
 }
@@ -231,7 +220,7 @@ TEST_F(AppCacheQuotaClientTest, GetOriginUsage) {
 
   SetUsageMapEntry(kOriginA, 1000);
   EXPECT_EQ(1000, GetOriginUsage(client, kOriginA, kTemp));
-  EXPECT_EQ(0, GetOriginUsage(client, kOriginA, kPerm));
+  EXPECT_EQ(0, GetOriginUsage(client, kOriginB, kTemp));
 
   Call_NotifyAppCacheDestroyed(client);
   Call_OnQuotaManagerDestroyed(client);
@@ -261,9 +250,6 @@ TEST_F(AppCacheQuotaClientTest, GetOriginsForHost) {
   EXPECT_EQ(1ul, origins.size());
   EXPECT_TRUE(origins.find(kOriginOther) != origins.end());
 
-  origins = GetOriginsForHost(client, kPerm, kOriginA.host());
-  EXPECT_TRUE(origins.empty());
-
   Call_NotifyAppCacheDestroyed(client);
   Call_OnQuotaManagerDestroyed(client);
 }
@@ -273,7 +259,6 @@ TEST_F(AppCacheQuotaClientTest, GetOriginsForType) {
   Call_NotifyAppCacheReady(client);
 
   EXPECT_TRUE(GetOriginsForType(client, kTemp).empty());
-  EXPECT_TRUE(GetOriginsForType(client, kPerm).empty());
 
   SetUsageMapEntry(kOriginA, 1000);
   SetUsageMapEntry(kOriginB, 10);
@@ -283,8 +268,6 @@ TEST_F(AppCacheQuotaClientTest, GetOriginsForType) {
   EXPECT_TRUE(origins.find(kOriginA) != origins.end());
   EXPECT_TRUE(origins.find(kOriginB) != origins.end());
 
-  EXPECT_TRUE(GetOriginsForType(client, kPerm).empty());
-
   Call_NotifyAppCacheDestroyed(client);
   Call_OnQuotaManagerDestroyed(client);
 }
@@ -293,12 +276,6 @@ TEST_F(AppCacheQuotaClientTest, DeleteOriginData) {
   auto client = CreateClient();
   Call_NotifyAppCacheReady(client);
 
-  // Perm deletions are short circuited in the Client and
-  // should not reach the AppCacheServiceImpl.
-  EXPECT_EQ(blink::mojom::QuotaStatusCode::kOk,
-            DeleteOriginData(client, kPerm, kOriginA));
-  EXPECT_EQ(0, mock_service_.delete_called_count());
-
   EXPECT_EQ(blink::mojom::QuotaStatusCode::kOk,
             DeleteOriginData(client, kTemp, kOriginA));
   EXPECT_EQ(1, mock_service_.delete_called_count());
@@ -320,15 +297,14 @@ TEST_F(AppCacheQuotaClientTest, PendingRequests) {
   SetUsageMapEntry(kOriginB, 10);
   SetUsageMapEntry(kOriginOther, 500);
 
-  // Queue up some reqeusts.
-  AsyncGetOriginUsage(client, kOriginA, kPerm);
+  // Queue up some requests.
+  AsyncGetOriginUsage(client, kOriginA, kTemp);
   AsyncGetOriginUsage(client, kOriginB, kTemp);
-  AsyncGetOriginsForType(client, kPerm);
+  AsyncGetOriginsForType(client, kTemp);
   AsyncGetOriginsForType(client, kTemp);
   AsyncGetOriginsForHost(client, kTemp, kOriginA.host());
   AsyncGetOriginsForHost(client, kTemp, kOriginOther.host());
   AsyncDeleteOriginData(client, kTemp, kOriginA);
-  AsyncDeleteOriginData(client, kPerm, kOriginA);
   AsyncDeleteOriginData(client, kTemp, kOriginB);
 
   EXPECT_EQ(0, num_get_origin_usage_completions_);
@@ -344,7 +320,7 @@ TEST_F(AppCacheQuotaClientTest, PendingRequests) {
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(2, num_get_origin_usage_completions_);
   EXPECT_EQ(4, num_get_origins_completions_);
-  EXPECT_EQ(3, num_delete_origins_completions_);
+  EXPECT_EQ(2, num_delete_origins_completions_);
 
   // They should be serviced in order requested.
   EXPECT_EQ(10, usage_);
@@ -362,15 +338,14 @@ TEST_F(AppCacheQuotaClientTest, DestroyServiceWithPending) {
   SetUsageMapEntry(kOriginB, 10);
   SetUsageMapEntry(kOriginOther, 500);
 
-  // Queue up some reqeusts prior to being ready.
-  AsyncGetOriginUsage(client, kOriginA, kPerm);
+  // Queue up some requests prior to being ready.
+  AsyncGetOriginUsage(client, kOriginA, kTemp);
   AsyncGetOriginUsage(client, kOriginB, kTemp);
-  AsyncGetOriginsForType(client, kPerm);
+  AsyncGetOriginsForType(client, kTemp);
   AsyncGetOriginsForType(client, kTemp);
   AsyncGetOriginsForHost(client, kTemp, kOriginA.host());
   AsyncGetOriginsForHost(client, kTemp, kOriginOther.host());
   AsyncDeleteOriginData(client, kTemp, kOriginA);
-  AsyncDeleteOriginData(client, kPerm, kOriginA);
   AsyncDeleteOriginData(client, kTemp, kOriginB);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(0, num_get_origin_usage_completions_);
@@ -383,7 +358,7 @@ TEST_F(AppCacheQuotaClientTest, DestroyServiceWithPending) {
   // All should have been aborted and called completion.
   EXPECT_EQ(2, num_get_origin_usage_completions_);
   EXPECT_EQ(4, num_get_origins_completions_);
-  EXPECT_EQ(3, num_delete_origins_completions_);
+  EXPECT_EQ(2, num_delete_origins_completions_);
   EXPECT_EQ(0, usage_);
   EXPECT_TRUE(origins_.empty());
   EXPECT_EQ(blink::mojom::QuotaStatusCode::kErrorAbort, delete_status_);
@@ -398,15 +373,14 @@ TEST_F(AppCacheQuotaClientTest, DestroyQuotaManagerWithPending) {
   SetUsageMapEntry(kOriginB, 10);
   SetUsageMapEntry(kOriginOther, 500);
 
-  // Queue up some reqeusts prior to being ready.
-  AsyncGetOriginUsage(client, kOriginA, kPerm);
+  // Queue up some requests prior to being ready.
+  AsyncGetOriginUsage(client, kOriginA, kTemp);
   AsyncGetOriginUsage(client, kOriginB, kTemp);
-  AsyncGetOriginsForType(client, kPerm);
+  AsyncGetOriginsForType(client, kTemp);
   AsyncGetOriginsForType(client, kTemp);
   AsyncGetOriginsForHost(client, kTemp, kOriginA.host());
   AsyncGetOriginsForHost(client, kTemp, kOriginOther.host());
   AsyncDeleteOriginData(client, kTemp, kOriginA);
-  AsyncDeleteOriginData(client, kPerm, kOriginA);
   AsyncDeleteOriginData(client, kTemp, kOriginB);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(0, num_get_origin_usage_completions_);