Provide API in RequestCoordinator to remove results by client ID.

components/offline_pages/background/request_queue_store_unittest.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/offline_pages/background/request_queue_store.h"
 
 #include <memory>
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
@@ skipping 274 matching lines @@
   store->RemoveRequests(request_ids,
                         base::Bind(&RequestQueueStoreTestBase::RemoveDone,
                                    base::Unretained(this)));
   ASSERT_EQ(LastResult::kNone, this->last_result());
   ASSERT_EQ(0, this->last_remove_count());
   this->PumpLoop();
   ASSERT_EQ(LastResult::kTrue, this->last_result());
   ASSERT_EQ(0, this->last_remove_count());
 }
 
+

[fgorski, 2016/08/02 05:01:23]

nit: remove empty line.

[Pete Williamson, 2016/08/02 23:27:17]

Done.

+TYPED_TEST(RequestQueueStoreTest, RemoveRequestByClientId) {

[fgorski, 2016/08/02 05:01:23]

I recommend testing with more than a single item, and as per earlier comments,
there are 2 variants of that:
* 2 different client IDs
* 2 requests with the same client ID.

[Pete Williamson, 2016/08/02 23:27:17]

Added new test, made this one take two different ids.

+  std::unique_ptr<RequestQueueStore> store(this->BuildStore());
+  base::Time creation_time = base::Time::Now();
+  SavePageRequest original_request(
+      kRequestId, kUrl, kClientId, creation_time, kUserRequested);
+  store->AddOrUpdateRequest(
+      original_request, base::Bind(&RequestQueueStoreTestBase::AddOrUpdateDone,
+                                   base::Unretained(this)));
+  this->PumpLoop();
+  this->ClearResults();
+
+  // For debugging only, remove next code paragraph before checkin.

[fgorski, 2016/08/02 05:01:23]

hmmm.

[Pete Williamson, 2016/08/02 23:27:17]

Removed (normally I use a TODO so I can spot these in the self-review process,
but I missed it this time.)  Thanks for the catch!

+  store->GetRequests(base::Bind(&RequestQueueStoreTestBase::GetRequestsDone,
+                                base::Unretained(this)));
+  this->PumpLoop();
+  this->ClearResults();
+
+  std::vector<ClientId> client_ids{kClientId};
+  store->RemoveRequestsByClientId(
+      client_ids,
+      base::Bind(&RequestQueueStoreTestBase::RemoveDone,
+                 base::Unretained(this)));
+  ASSERT_EQ(LastResult::kNone, this->last_result());
+  ASSERT_EQ(0, this->last_remove_count());
+  this->PumpLoop();
+  ASSERT_EQ(LastResult::kTrue, this->last_result());
+  ASSERT_EQ(1, this->last_remove_count());
+  this->ClearResults();
+
+  store->GetRequests(base::Bind(&RequestQueueStoreTestBase::GetRequestsDone,
+                                base::Unretained(this)));
+  this->PumpLoop();
+  ASSERT_EQ(LastResult::kTrue, this->last_result());
+  ASSERT_TRUE(this->last_requests().empty());
+  this->ClearResults();
+
+  // Removing a request that is missing fails.

[fgorski, 2016/08/02 05:01:23]

wow. Your copy-paste-fu is no good :)
Please fix the comment here and above. It is not  a failure, simply nothing to
delete.

[Pete Williamson, 2016/08/02 23:27:17]

Done.

+  store->RemoveRequestsByClientId(
+      client_ids,
+      base::Bind(&RequestQueueStoreTestBase::RemoveDone,
+                 base::Unretained(this)));
+  ASSERT_EQ(LastResult::kNone, this->last_result());
+  ASSERT_EQ(0, this->last_remove_count());
+  this->PumpLoop();
+  ASSERT_EQ(LastResult::kTrue, this->last_result());
+  ASSERT_EQ(0, this->last_remove_count());
+}
+

[fgorski, 2016/08/02 05:01:23]

nit: empty line.

[Pete Williamson, 2016/08/02 23:27:17]

Done.

+
 TYPED_TEST(RequestQueueStoreTest, ResetStore) {
   std::unique_ptr<RequestQueueStore> store(this->BuildStore());
   base::Time creation_time = base::Time::Now();
   SavePageRequest original_request(
       kRequestId, kUrl, kClientId, creation_time, kUserRequested);
   store->AddOrUpdateRequest(
       original_request, base::Bind(&RequestQueueStoreTestBase::AddOrUpdateDone,
                                    base::Unretained(this)));
   this->PumpLoop();
   this->ClearResults();
@@ skipping 35 matching lines @@
   store->GetRequests(base::Bind(&RequestQueueStoreTestBase::GetRequestsDone,
                                 base::Unretained(this)));
   ASSERT_EQ(LastResult::kNone, this->last_result());
   this->PumpLoop();
   ASSERT_EQ(LastResult::kTrue, this->last_result());
   ASSERT_EQ(1ul, this->last_requests().size());
   ASSERT_TRUE(original_request == this->last_requests()[0]);
 }
 
 }  // offline_pages