Implement important sites dialog for desktop.

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 <vector>
 
+#include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
+#include "base/values.h"
 #include "chrome/browser/browsing_data/browsing_data_counter_factory.h"
 #include "chrome/browser/browsing_data/browsing_data_counter_utils.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
+#include "chrome/browser/browsing_data/browsing_data_important_sites_util.h"
 #include "chrome/browser/browsing_data/browsing_data_remover_factory.h"
 #include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h"
+#include "chrome/browser/engagement/important_sites_util.h"
 #include "chrome/browser/history/web_history_service_factory.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/common/channel_info.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/pref_names.h"
 #include "components/browsing_data/core/history_notice_utils.h"
 #include "components/browsing_data/core/pref_names.h"
 #include "components/prefs/pref_service.h"
+#include "content/public/browser/browsing_data_filter_builder.h"
 #include "content/public/browser/web_ui.h"
+#include "ui/base/text/bytes_formatting.h"
+
+using ImportantReason = ImportantSitesUtil::ImportantReason;
 
 namespace {
 
 const int kMaxTimesHistoryNoticeShown = 1;
 
+const int kMaxImportantSites = 10;
+
 // TODO(msramek): Get the list of deletion preferences from the JS side.
 const char* kCounterPrefs[] = {
   browsing_data::prefs::kDeleteBrowsingHistory,
   browsing_data::prefs::kDeleteCache,
   browsing_data::prefs::kDeleteDownloadHistory,
   browsing_data::prefs::kDeleteFormData,
   browsing_data::prefs::kDeleteHostedAppsData,
   browsing_data::prefs::kDeleteMediaLicenses,
   browsing_data::prefs::kDeletePasswords,
 };
 
+const char kRegisterableDomainField[] = "registerableDomain";
+const char kReasonBitField[] = "reasonBitfield";
+const char kExampleOriginField[] = "exampleOrigin";
+const char kFlagEnabledField[] = "flagEnabled";
+const char kImportantSitesField[] = "importantSites";
+const char kIsCheckedField[] = "isChecked";
+const char kStorageSizeField[] = "storageSize";
+const char kHasNotificationsField[] = "hasNotifications";
+
 } // namespace
 
 namespace settings {
 
 // TaskObserver ----------------------------------------------------------------
 
 class ClearBrowsingDataHandler::TaskObserver
     : public BrowsingDataRemover::Observer {
  public:
-  TaskObserver(BrowsingDataRemover* remover, const base::Closure& callback);
+  TaskObserver(BrowsingDataRemover* remover,
+               const base::Closure& callback,
+               int task_count);
   ~TaskObserver() override;
 
   void OnBrowsingDataRemoverDone() override;
 
  private:
   base::Closure callback_;
   ScopedObserver<BrowsingDataRemover, BrowsingDataRemover::Observer>
       remover_observer_;
+  int task_count_;
 
   DISALLOW_COPY_AND_ASSIGN(TaskObserver);
 };
 
 ClearBrowsingDataHandler::TaskObserver::TaskObserver(
-    BrowsingDataRemover* remover, const base::Closure& callback)
-    : callback_(callback),
-      remover_observer_(this) {
+    BrowsingDataRemover* remover,
+    const base::Closure& callback,
+    int task_count)
+    : callback_(callback), remover_observer_(this), task_count_(task_count) {
   remover_observer_.Add(remover);
 }
 
 ClearBrowsingDataHandler::TaskObserver::~TaskObserver() {}
 
 void ClearBrowsingDataHandler::TaskObserver::OnBrowsingDataRemoverDone() {
+  DCHECK(task_count_);
+  if (--task_count_)
+    return;
   remover_observer_.RemoveAll();
   callback_.Run();
 }
 
 // ClearBrowsingDataHandler ----------------------------------------------------
 
 ClearBrowsingDataHandler::ClearBrowsingDataHandler(content::WebUI* webui)
     : profile_(Profile::FromWebUI(webui)),
       sync_service_(ProfileSyncServiceFactory::GetForProfile(profile_)),
       sync_service_observer_(this),
       show_history_footer_(false),
       show_history_deletion_dialog_(false),
       weak_ptr_factory_(this) {}
 
 ClearBrowsingDataHandler::~ClearBrowsingDataHandler() {
 }
 
 void ClearBrowsingDataHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
       "clearBrowsingData",
       base::Bind(&ClearBrowsingDataHandler::HandleClearBrowsingData,
                  base::Unretained(this)));
 
   web_ui()->RegisterMessageCallback(
+      "fetchImportantSites",
+      base::Bind(&ClearBrowsingDataHandler::HandleFetchImportantSites,
+                 base::Unretained(this)));
+
+  web_ui()->RegisterMessageCallback(
       "initializeClearBrowsingData",
       base::Bind(&ClearBrowsingDataHandler::HandleInitialize,
                  base::Unretained(this)));
 }
 
 void ClearBrowsingDataHandler::OnJavascriptAllowed() {
   if (sync_service_)
     sync_service_observer_.Add(sync_service_);
 
   DCHECK(counters_.empty());
@@ skipping 83 matching lines @@
         [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);
 
   std::string webui_callback_id;
-  CHECK_EQ(1U, args->GetSize());
+  CHECK_EQ(2U, args->GetSize());
   CHECK(args->GetString(0, &webui_callback_id));
 
+  std::vector<std::string> excluding_domains;
+  std::vector<int32_t> excluding_domain_reasons;
+  std::vector<std::string> ignoring_domains;
+  std::vector<int32_t> ignoring_domain_reasons;
+  const base::ListValue* important_sites = nullptr;
+  CHECK(args->GetList(1, &important_sites));
+  for (const auto& item : *important_sites) {
+    const base::DictionaryValue* site = nullptr;
+    CHECK(item->GetAsDictionary(&site));
+    bool is_checked = false;
+    CHECK(site->GetBoolean(kIsCheckedField, &is_checked));
+    std::string domain;
+    int domain_reason = -1;
+    CHECK(site->GetString(kRegisterableDomainField, &domain));
+    CHECK(site->GetInteger(kReasonBitField, &domain_reason));
+    if (is_checked) {  // Selected important sites should be deleted.
+      ignoring_domains.push_back(domain);
+      ignoring_domain_reasons.push_back(domain_reason);
+    } else {  // Unselected sites should be kept.
+      excluding_domains.push_back(domain);
+      excluding_domain_reasons.push_back(domain_reason);
+    }
+  }
+
+  std::unique_ptr<content::BrowsingDataFilterBuilder> filter_builder(
+      content::BrowsingDataFilterBuilder::Create(
+          content::BrowsingDataFilterBuilder::BLACKLIST));
+  for (const std::string& domain : excluding_domains) {
+    filter_builder->AddRegisterableDomain(domain);
+  }
+
+  if (!excluding_domains.empty() || !ignoring_domains.empty()) {
+    ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
+        profile_->GetOriginalProfile(), excluding_domains,
+        excluding_domain_reasons, ignoring_domains, ignoring_domain_reasons);
+  }
+
   BrowsingDataRemover* remover =
       BrowsingDataRemoverFactory::GetForBrowserContext(profile_);
   task_observer_ = base::MakeUnique<TaskObserver>(
       remover,
       base::Bind(&ClearBrowsingDataHandler::OnClearingTaskFinished,
-                 base::Unretained(this), webui_callback_id));
+                 base::Unretained(this), webui_callback_id),
+      /*task_count=*/2);
   browsing_data::TimePeriod time_period =
       static_cast<browsing_data::TimePeriod>(period_selected);
-  browsing_data::RecordDeletionForPeriod(time_period);
-  remover->RemoveAndReply(
-      browsing_data::CalculateBeginDeleteTime(time_period),
-      browsing_data::CalculateEndDeleteTime(time_period),
-      remove_mask, origin_mask, task_observer_.get());
+
+  browsing_data_important_sites_util::Remove(
+      remove_mask, origin_mask, time_period, std::move(filter_builder), remover,
+      task_observer_.get());
 }
 
 void ClearBrowsingDataHandler::OnClearingTaskFinished(
     const std::string& webui_callback_id) {
   PrefService* prefs = profile_->GetPrefs();
   int notice_shown_times = prefs->GetInteger(
       browsing_data::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
@@ skipping 13 matching lines @@
   }
 
   UMA_HISTOGRAM_BOOLEAN(
       "History.ClearBrowsingData.ShownHistoryNoticeAfterClearing", show_notice);
 
   ResolveJavascriptCallback(base::Value(webui_callback_id),
                             base::Value(show_notice));
   task_observer_.reset();
 }
 
+void ClearBrowsingDataHandler::HandleFetchImportantSites(
+    const base::ListValue* args) {
+  AllowJavascript();
+  const base::Value* callback_id;
+  CHECK(args->Get(0, &callback_id));
+
+  Profile* profile = profile_->GetOriginalProfile();
+  bool flag_enabled =
+      base::FeatureList::IsEnabled(features::kImportantSitesInCbd);
+  bool dialog_disabled = ImportantSitesUtil::IsDialogDisabled(profile);

[dschuyler, 2017/04/11 23:49:32]

Please avoid the generic terminology / var names here: what is the 'dialog'.
I'm guessing it may be talking about the dialog that that CBD (clear browsing
data) is shown in, but this code shouldn't be so tied to the UI implementation
(like whether it's a dialog or something else). (If that's not what it's
referring to, then my comment stands because it's still unclear what dialog is
being discussed).

[dullweber, 2017/04/13 14:47:49]

I added "important_sites_" to both of these bools to clarify that they are about
the state of the dialog showing important sites, which will be shown after a
user chooses to clear browsing data.

+  auto important_sites_list = base::MakeUnique<base::ListValue>();
+
+  if (flag_enabled && !dialog_disabled) {
+    std::vector<ImportantSitesUtil::ImportantDomainInfo> important_sites =
+        ImportantSitesUtil::GetImportantRegisterableDomains(profile,
+                                                            kMaxImportantSites);
+    for (const auto& info : important_sites) {
+      auto entry = base::MakeUnique<base::DictionaryValue>();
+      entry->SetString(kRegisterableDomainField, info.registerable_domain);
+      entry->SetInteger(kReasonBitField, info.reason_bitfield);

[dschuyler, 2017/04/11 23:49:32]

I'm iffy on whether a bit field is a good data type to use between C++ and JS.
Please tell me more about why this is a good choice (like there being precedence
for it; or a spec entry saying that the bit manipulation operations will be
consistent). 

My concerns come from bit fields being handled inconsistently by different
languages (or compilers of the same language) in the past. (Twenty years ago
passing a bit field between environments would have been a risky thing to do;
I'm trying to stay open to hearing whether this has changed - or isn't an issue
in this specific case).

[dullweber, 2017/04/13 14:47:50]

The reason_bitfield is only accessed here and in
ClearBrowsingdataHandler::HandleClearBrowsingData() to call
ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites. It is never
accessed from Javascript. 
The only bit operation is done in line 345 to extract, whether this site has
notifications enabled.
The only expectation is that the integer will survive being passed to Javascript
and back. reason_bitfield is an int32_t and Javascript should support 53bit
precision for whole numbers. If that wouldn't work correctly, it would mean that
any integer could change when beeing passed to Javascript? 
The same is done on the Java side when passing this bitfield to the Android UI
and back.

+      entry->SetString(kExampleOriginField, info.example_origin.spec());
+      // Initially all sites are selected for deletion.
+      entry->SetBoolean(kIsCheckedField, true);
+      // TODO(dullweber): Get size.
+      entry->SetString(kStorageSizeField, ui::FormatBytes(0));
+      bool has_notifications =
+          (info.reason_bitfield & (1 << ImportantReason::NOTIFICATIONS)) != 0;
+      entry->SetBoolean(kHasNotificationsField, has_notifications);
+      important_sites_list->Append(std::move(entry));
+    }
+  }
+
+  std::unique_ptr<base::DictionaryValue> result(new base::DictionaryValue());
+  result->SetBoolean(kFlagEnabledField, flag_enabled);
+  result->Set(kImportantSitesField, std::move(important_sites_list));
+
+  ResolveJavascriptCallback(*callback_id, *result);
+}
+
 void ClearBrowsingDataHandler::HandleInitialize(const base::ListValue* args) {
   AllowJavascript();
   const base::Value* callback_id;
   CHECK(args->Get(0, &callback_id));
 
   // Needed because WebUI doesn't handle renderer crashes. See crbug.com/610450.
   task_observer_.reset();
 
   UpdateSyncState();
   RefreshHistoryNotice();
@@ skipping 64 matching lines @@
 
 void ClearBrowsingDataHandler::UpdateCounterText(
     std::unique_ptr<browsing_data::BrowsingDataCounter::Result> result) {
   CallJavascriptFunction(
       "cr.webUIListenerCallback", base::Value("update-counter-text"),
       base::Value(result->source()->GetPrefName()),
       base::Value(GetChromeCounterTextFromResult(result.get())));
 }
 
 }  // namespace settings