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 add59628a1efcfff772a03ff41565593d9cba286..0863c59e2ca2cb6e65de0ad3247120d72f18004b 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
@@ -46,7 +46,6 @@ 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),
@@ -77,25 +76,19 @@ 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();
 }
 
 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());
+  // We should never be called when the previous clearing from this dialog
+  // has not yet finished.
   CHECK(webui_callback_id_.empty());
+  CHECK_EQ(1U, args->GetSize());
   CHECK(args->GetString(0, &webui_callback_id_));
 
   PrefService* prefs = profile_->GetPrefs();
@@ -173,18 +166,21 @@ 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);
+
+  BrowsingDataRemover* remover =
+      BrowsingDataRemoverFactory::GetForBrowserContext(profile_);
+  DCHECK(remover);
+  remover_observer_.Add(remover);
+  remover->RemoveAndReply(
+      BrowsingDataRemover::Period(
+          static_cast<browsing_data::TimePeriod>(period_selected)),
+      remove_mask, origin_mask, this);
 }
 
-void ClearBrowsingDataHandler::OnBrowsingDataRemoving(bool is_removing) {
-  CallJavascriptFunction("cr.webUIListenerCallback",
-                         base::StringValue("browsing-data-removing"),
-                         base::FundamentalValue(is_removing));
+void ClearBrowsingDataHandler::OnBrowsingDataRemoverDone() {
+  remover_observer_.RemoveAll();
 
-  if (is_removing || webui_callback_id_.empty())
-    return;
+  DCHECK(!webui_callback_id_.empty());

[Dan Beam, 2016/08/16 22:09:08]

this could currently fail in the [very uncommon case of]:

1) start long clear
2) refresh
3) finish short clear

can we just clear this and only respond if it's not empty when the clearing
succeeds?

is it possible that this flow would have issues if the same observer was
re-added before the long clear succeeds?

 
   PrefService* prefs = profile_->GetPrefs();
   int notice_shown_times =
@@ -239,8 +235,9 @@ void ClearBrowsingDataHandler::HandleInitialize(const base::ListValue* args) {
   OnStateChanged();
   RefreshHistoryNotice();
 
-  ResolveJavascriptCallback(*callback_id,
-                            base::FundamentalValue(remover_->is_removing()));
+  ResolveJavascriptCallback(
+      *callback_id,
+      base::FundamentalValue(!webui_callback_id_.empty()));
 }
 
 void ClearBrowsingDataHandler::OnStateChanged() {
@@ -284,7 +281,7 @@ void ClearBrowsingDataHandler::UpdateHistoryNotice(bool show) {
 }
 
 void ClearBrowsingDataHandler::UpdateHistoryDeletionDialog(bool show) {
-  // This is used by OnBrowsingDataRemoving (when the deletion finishes).
+  // This is used by OnBrowsingDataRemoverDone (when the deletion finishes).
   show_history_deletion_dialog_ = show;
 }