Make ClearBrowsingDataHandler only observe its own removal task

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

 // Copyright 2015 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 "chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.h"
 
 #include <stddef.h>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
@@ skipping 28 matching lines @@
 };
 
 } // namespace
 
 namespace settings {
 
 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) {}
 
 ClearBrowsingDataHandler::~ClearBrowsingDataHandler() {
 }
 
 void ClearBrowsingDataHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
@@ skipping 10 matching lines @@
 void ClearBrowsingDataHandler::OnJavascriptAllowed() {
   PrefService* prefs = profile_->GetPrefs();
   profile_pref_registrar_.Init(prefs);
   profile_pref_registrar_.Add(
       prefs::kAllowDeletingBrowserHistory,
       base::Bind(&ClearBrowsingDataHandler::OnBrowsingHistoryPrefChanged,
                  base::Unretained(this)));
 
   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());
+  // 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(webui_callback_id_.empty());
   CHECK(args->GetString(0, &webui_callback_id_));
 
   PrefService* prefs = profile_->GetPrefs();
 
   int site_data_mask = BrowsingDataRemover::REMOVE_SITE_DATA;
   // Don't try to clear LSO data if it's not supported.
   if (!prefs->GetBoolean(prefs::kClearPluginLSODataEnabled))
     site_data_mask &= ~BrowsingDataRemover::REMOVE_PLUGIN_DATA;
 
   int remove_mask = 0;
@@ skipping 57 matching lines @@
     int checked_other_types = std::count_if(
         other_types, other_types + num_other_types,
         [prefs](const std::string& pref) { return prefs->GetBoolean(pref); });
     UMA_HISTOGRAM_SPARSE_SLOWLY(
         "History.ClearBrowsingData.PasswordsDeletion.AdditionalDatatypesCount",
         checked_other_types);
   }
 
   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 =
       prefs->GetInteger(prefs::kClearBrowsingDataHistoryNoticeShownTimes);
 
   // When the deletion is complete, we might show an additional dialog with
   // a notice about other forms of browsing history. This is the case if
   const bool show_notice =
       // 1. The dialog is relevant for the user.
       show_history_deletion_dialog_ &&
@@ skipping 34 matching lines @@
   counters_.clear();
 
   for (const std::string& pref : kCounterPrefs) {
     AddCounter(
         BrowsingDataCounterFactory::GetForProfileAndPref(profile_, pref));
   }
 
   OnStateChanged();
   RefreshHistoryNotice();
 
-  ResolveJavascriptCallback(*callback_id,
-                            base::FundamentalValue(remover_->is_removing()));
+  ResolveJavascriptCallback(
+      *callback_id,
+      base::FundamentalValue(!webui_callback_id_.empty()));
 }
 
 void ClearBrowsingDataHandler::OnStateChanged() {
   CallJavascriptFunction(
       "cr.webUIListenerCallback",
       base::StringValue("update-footer"),
       base::FundamentalValue(sync_service_ && sync_service_->IsSyncActive()),
       base::FundamentalValue(show_history_footer_));
 }
 
@@ skipping 23 matching lines @@
 void ClearBrowsingDataHandler::UpdateHistoryNotice(bool show) {
   show_history_footer_ = show;
   OnStateChanged();
 
   UMA_HISTOGRAM_BOOLEAN(
       "History.ClearBrowsingData.HistoryNoticeShownInFooterWhenUpdated",
       show_history_footer_);
 }
 
 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;
 }
 
 void ClearBrowsingDataHandler::AddCounter(
     std::unique_ptr<browsing_data::BrowsingDataCounter> counter) {
   counter->Init(profile_->GetPrefs(),
                 base::Bind(&ClearBrowsingDataHandler::UpdateCounterText,
                            base::Unretained(this)));
   counter->Restart();
   counters_.push_back(std::move(counter));
 }
 
 void ClearBrowsingDataHandler::UpdateCounterText(
     std::unique_ptr<browsing_data::BrowsingDataCounter::Result> result) {
   CallJavascriptFunction(
       "cr.webUIListenerCallback",
       base::StringValue("update-counter-text"),
       base::StringValue(result->source()->GetPrefName()),
       base::StringValue(GetCounterTextFromResult(result.get())));
 }
 
 }  // namespace settings