Move BrowsingDataRemoverImpl:: CompletionInhibitor to the public interface

chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc

 // Copyright 2017 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 <stddef.h>
 #include <stdint.h>
 
 #include <list>
 #include <memory>
 #include <set>
@@ skipping 1420 matching lines @@
   void OnBrowsingDataRemoverDone() override {
     BrowsingDataRemoverCompletionObserver::OnBrowsingDataRemoverDone();
     called_ = true;
   }
 
  private:
   bool called_ = false;
 };
 
 TEST_F(BrowsingDataRemoverImplTest, CompletionInhibition) {
+  BrowsingDataRemoverImpl* remover = static_cast<BrowsingDataRemoverImpl*>(
+      BrowsingDataRemoverFactory::GetForBrowserContext(GetBrowserContext()));
+
   // The |completion_inhibitor| on the stack should prevent removal sessions
   // from completing until after ContinueToCompletion() is called.
-  BrowsingDataRemoverCompletionInhibitor completion_inhibitor;
-
-  BrowsingDataRemoverImpl* remover = static_cast<BrowsingDataRemoverImpl*>(
-      BrowsingDataRemoverFactory::GetForBrowserContext(GetBrowserContext()));
+  BrowsingDataRemoverCompletionInhibitor completion_inhibitor(remover);
   InspectableCompletionObserver completion_observer(remover);
   remover->RemoveAndReply(
       base::Time(), base::Time::Max(), BrowsingDataRemover::DATA_TYPE_COOKIES,
       BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB, &completion_observer);
 
   // Process messages until the inhibitor is notified, and then some, to make
   // sure we do not complete asynchronously before ContinueToCompletion() is
   // called.
   completion_inhibitor.BlockUntilNearCompletion();
   base::RunLoop().RunUntilIdle();
 
   // Verify that the removal has not yet been completed and the observer has
   // not been called.
   EXPECT_TRUE(remover->is_removing());
   EXPECT_FALSE(completion_observer.called());
 
   // Now run the removal process until completion, and verify that observers are
   // now notified, and the notifications is sent out.
   completion_inhibitor.ContinueToCompletion();
   completion_observer.BlockUntilCompletion();
 
   EXPECT_FALSE(remover->is_removing());
   EXPECT_TRUE(completion_observer.called());
 }
 
 TEST_F(BrowsingDataRemoverImplTest, EarlyShutdown) {
   BrowsingDataRemoverImpl* remover = static_cast<BrowsingDataRemoverImpl*>(
       BrowsingDataRemoverFactory::GetForBrowserContext(GetBrowserContext()));
   InspectableCompletionObserver completion_observer(remover);
-  BrowsingDataRemoverCompletionInhibitor completion_inhibitor;
+  BrowsingDataRemoverCompletionInhibitor completion_inhibitor(remover);
   remover->RemoveAndReply(
       base::Time(), base::Time::Max(), BrowsingDataRemover::DATA_TYPE_COOKIES,
       BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB, &completion_observer);
 
   completion_inhibitor.BlockUntilNearCompletion();
 
   // Verify that the deletion has not yet been completed and the observer has
   // not been called.
   EXPECT_TRUE(remover->is_removing());
   EXPECT_FALSE(completion_observer.called());
 
   // Destroying the profile should trigger the notification.
   DestroyBrowserContext();
 
   EXPECT_TRUE(completion_observer.called());
 
   // Finishing after shutdown shouldn't break anything.
   completion_inhibitor.ContinueToCompletion();

[Bernhard Bauer, 2017/04/12 16:35:34]

You could still keep these if you remove the DCHECK(remover_) in
BrowsingDataRemoverCompletionInhibitor::ContinueToCompletion, right? That might
not be a bad test – verifying that the completion callback can be called even
after the BrowsingDataRemover has been destroyed.

(If you don't want to keep this, turns out that you don't use the
|completion_inhibitor| after you reset it, so you could just destroy it at that
point! Then you wouldn't even need an explicit Reset() method anymore.)

[msramek, 2017/04/12 19:16:55]

Sigh... I forgot that |continue_to_completion_callback_| still has a weak
pointer bound in, so it still can be called. Reverted this part.

   completion_observer.BlockUntilCompletion();
 }
 
 TEST_F(BrowsingDataRemoverImplTest, RemoveDownloadsByTimeOnly) {
   RemoveDownloadsTester tester(GetBrowserContext());
   base::Callback<bool(const GURL&)> filter =
       BrowsingDataFilterBuilder::BuildNoopFilter();
 
   EXPECT_CALL(
       *tester.download_manager(),
@@ skipping 78 matching lines @@
       BrowsingDataRemoverFactory::GetForBrowserContext(GetBrowserContext()));
   EXPECT_FALSE(remover->is_removing());
 
   std::unique_ptr<BrowsingDataFilterBuilder> filter_builder_1(
       BrowsingDataFilterBuilder::Create(BrowsingDataFilterBuilder::WHITELIST));
   std::unique_ptr<BrowsingDataFilterBuilder> filter_builder_2(
       BrowsingDataFilterBuilder::Create(BrowsingDataFilterBuilder::BLACKLIST));
   filter_builder_2->AddRegisterableDomain("example.com");
 
   MultipleTasksObserver observer(remover);
-  BrowsingDataRemoverCompletionInhibitor completion_inhibitor;
+  BrowsingDataRemoverCompletionInhibitor completion_inhibitor(remover);
 
   // Test several tasks with various configuration of masks, filters, and target
   // observers.
   std::list<BrowsingDataRemoverImpl::RemovalTask> tasks;
   tasks.emplace_back(
       base::Time(), base::Time::Max(), BrowsingDataRemover::DATA_TYPE_DOWNLOADS,
       BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB,
       BrowsingDataFilterBuilder::Create(BrowsingDataFilterBuilder::BLACKLIST),
       observer.target_a());
   tasks.emplace_back(
@@ skipping 102 matching lines @@
 
   EXPECT_TRUE(remover->is_removing());
 
   // Add one more deletion and wait for it.
   BlockUntilBrowsingDataRemoved(
       base::Time(), base::Time::Max(), BrowsingDataRemover::DATA_TYPE_COOKIES,
       BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB);
 
   EXPECT_FALSE(remover->is_removing());
 }