Make ClearBrowsingDataHandler only observe its own removal task

chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc

Index: chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc
diff --git a/chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc b/chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc
index 0835756a187d7dd8b42219c8698248eaef4e14db..06df7c80f127fa263c3d70abe096d5b2ce5a74cd 100644
--- a/chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc
+++ b/chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc
@@ -42,12 +42,44 @@ const char* kCounterPrefs[] = {
 
 namespace settings {
 
+// TaskObserver ----------------------------------------------------------------
+
+class ClearBrowsingDataHandler::TaskObserver
+    : public BrowsingDataRemover::Observer {
+ public:
+  TaskObserver(BrowsingDataRemover* remover, const base::Closure& callback);
+  ~TaskObserver() override;
+
+  void OnBrowsingDataRemoverDone() override;
+
+ private:
+  base::Closure callback_;
+  ScopedObserver<BrowsingDataRemover, BrowsingDataRemover::Observer>
+      remover_observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(TaskObserver);
+};
+
+ClearBrowsingDataHandler::TaskObserver::TaskObserver(
+    BrowsingDataRemover* remover, const base::Closure& callback)
+    : callback_(callback),
+      remover_observer_(this) {
+  remover_observer_.Add(remover);
+}
+
+ClearBrowsingDataHandler::TaskObserver::~TaskObserver() {}
+
+void ClearBrowsingDataHandler::TaskObserver::OnBrowsingDataRemoverDone() {
+  remover_observer_.RemoveAll();
+  callback_.Run();
+}

[Dan Beam, 2016/09/01 15:49:32]

fwiw: i usually find inline function bodies better to read if the interface and
implementation are in the same file, i.e.

TaskObserver(BrowsingDataRemover* remover, const base::Closure& callback)
    : callback_(callback), remover_observer_(this) {}
~TaskObserver() override {}

void OnBrowsingDataRemoverDone() {
  remover_observer_.RemoveAll();
  callback_.Run();
}

+
+// ClearBrowsingDataHandler ----------------------------------------------------
+
 ClearBrowsingDataHandler::ClearBrowsingDataHandler(content::WebUI* webui)
     : profile_(Profile::FromWebUI(webui)),
       sync_service_(ProfileSyncServiceFactory::GetForProfile(profile_)),
       sync_service_observer_(this),
-      remover_(nullptr),
-      remover_observer_(this),
       show_history_footer_(false),
       show_history_deletion_dialog_(false),
       weak_ptr_factory_(this) {}
@@ -77,26 +109,17 @@ void ClearBrowsingDataHandler::OnJavascriptAllowed() {
 
   if (sync_service_)
     sync_service_observer_.Add(sync_service_);
-
-  if (!remover_)
-    remover_ = BrowsingDataRemoverFactory::GetForBrowserContext(profile_);
-
-  remover_observer_.Add(remover_);
 }
 
 void ClearBrowsingDataHandler::OnJavascriptDisallowed() {
-  remover_observer_.RemoveAll();
   profile_pref_registrar_.RemoveAll();
   sync_service_observer_.RemoveAll();
+  task_observer_.reset();
 }
 
 void ClearBrowsingDataHandler::HandleClearBrowsingData(
     const base::ListValue* args) {
-  // We should never be called when the previous clearing has not yet finished.
-  CHECK(!remover_->is_removing());
-  CHECK_EQ(1U, args->GetSize());
-  CHECK(webui_callback_id_.empty());
-  CHECK(args->GetString(0, &webui_callback_id_));
+  DCHECK(!task_observer_);
 
   PrefService* prefs = profile_->GetPrefs();
 
@@ -173,19 +196,25 @@ void ClearBrowsingDataHandler::HandleClearBrowsingData(
 
   int period_selected =
       prefs->GetInteger(browsing_data::prefs::kDeleteTimePeriod);
-  remover_->Remove(BrowsingDataRemover::Period(
-                       static_cast<browsing_data::TimePeriod>(period_selected)),
-                   remove_mask, origin_mask);
-}
-
-void ClearBrowsingDataHandler::OnBrowsingDataRemoving(bool is_removing) {
-  CallJavascriptFunction("cr.webUIListenerCallback",
-                         base::StringValue("browsing-data-removing"),
-                         base::FundamentalValue(is_removing));
 
-  if (is_removing || webui_callback_id_.empty())
-    return;
+  std::string webui_callback_id;
+  CHECK_EQ(1U, args->GetSize());
+  CHECK(args->GetString(0, &webui_callback_id));
+
+  BrowsingDataRemover* remover =
+      BrowsingDataRemoverFactory::GetForBrowserContext(profile_);
+  task_observer_ = base::MakeUnique<TaskObserver>(
+      remover,
+      base::Bind(&ClearBrowsingDataHandler::OnClearingTaskFinished,
+                 base::Unretained(this), webui_callback_id));
+  remover->RemoveAndReply(
+      BrowsingDataRemover::Period(
+          static_cast<browsing_data::TimePeriod>(period_selected)),
+      remove_mask, origin_mask, task_observer_.get());
+}
 
+void ClearBrowsingDataHandler::OnClearingTaskFinished(
+    const std::string& webui_callback_id) {
   PrefService* prefs = profile_->GetPrefs();
   int notice_shown_times =
       prefs->GetInteger(prefs::kClearBrowsingDataHistoryNoticeShownTimes);
@@ -210,9 +239,9 @@ void ClearBrowsingDataHandler::OnBrowsingDataRemoving(bool is_removing) {
       "History.ClearBrowsingData.ShownHistoryNoticeAfterClearing", show_notice);
 
   ResolveJavascriptCallback(
-      base::StringValue(webui_callback_id_),
+      base::StringValue(webui_callback_id),
       base::FundamentalValue(show_notice));
-  webui_callback_id_.clear();
+  task_observer_.reset();
 }
 
 void ClearBrowsingDataHandler::OnBrowsingHistoryPrefChanged() {
@@ -229,6 +258,7 @@ void ClearBrowsingDataHandler::HandleInitialize(const base::ListValue* args) {
   const base::Value* callback_id;
   CHECK(args->Get(0, &callback_id));
 
+  task_observer_.reset();
   counters_.clear();
 
   for (const std::string& pref : kCounterPrefs) {
@@ -239,8 +269,9 @@ void ClearBrowsingDataHandler::HandleInitialize(const base::ListValue* args) {
   OnStateChanged();
   RefreshHistoryNotice();
 
-  ResolveJavascriptCallback(*callback_id,
-                            base::FundamentalValue(remover_->is_removing()));
+  ResolveJavascriptCallback(
+      *callback_id,
+      *base::Value::CreateNullValue() /* Promise<void> */);
 }
 
 void ClearBrowsingDataHandler::OnStateChanged() {
@@ -284,7 +315,7 @@ void ClearBrowsingDataHandler::UpdateHistoryNotice(bool show) {
 }
 
 void ClearBrowsingDataHandler::UpdateHistoryDeletionDialog(bool show) {
-  // This is used by OnBrowsingDataRemoving (when the deletion finishes).
+  // This is used by OnClearingTaskFinished (when the deletion finishes).
   show_history_deletion_dialog_ = show;
 }