MD History: update (instead of re-create) data sources on sign in change

chrome/browser/ui/webui/md_history_ui.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/md_history_ui.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
+#include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/ui/webui/browsing_history_handler.h"
 #include "chrome/browser/ui/webui/foreign_session_handler.h"
 #include "chrome/browser/ui/webui/history_login_handler.h"
 #include "chrome/browser/ui/webui/metrics_handler.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/browser_resources.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/locale_settings.h"
 #include "chrome/grit/theme_resources.h"
 #include "components/grit/components_scaled_resources.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/search/search.h"
 #include "components/signin/core/browser/signin_manager.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/web_ui.h"
 #include "content/public/browser/web_ui_data_source.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 
 namespace {
 
+constexpr char kIsUserSignedInKey[] = "isUserSignedIn";
+constexpr char kShowMenuPromoKey[] = "showMenuPromo";
+
+bool IsUserSignedIn(Profile* profile) {
+  SigninManagerBase* signin_manager =
+      SigninManagerFactory::GetForProfile(profile);
+  return signin_manager && signin_manager->IsAuthenticated();
+}
+
+bool MenuPromoShown(Profile* profile) {
+  return profile->GetPrefs()->GetBoolean(prefs::kMdHistoryMenuPromoShown);
+}
+
 content::WebUIDataSource* CreateMdHistoryUIHTMLSource(Profile* profile,
                                                       bool use_test_title) {
   content::WebUIDataSource* source =
       content::WebUIDataSource::Create(chrome::kChromeUIHistoryHost);
 
   // Localized strings (alphabetical order).
   source->AddLocalizedString("bookmarked", IDS_HISTORY_ENTRY_BOOKMARKED);
   source->AddLocalizedString("cancel", IDS_CANCEL);
   source->AddLocalizedString("clearBrowsingData",
                              IDS_CLEAR_BROWSING_DATA_TITLE);
@@ skipping 62 matching lines @@
       l10n_util::GetStringFUTF16(
           IDS_HISTORY_OTHER_FORMS_OF_HISTORY,
           l10n_util::GetStringUTF16(
               IDS_SETTINGS_CLEAR_DATA_WEB_HISTORY_URL_IN_HISTORY)));
 
   PrefService* prefs = profile->GetPrefs();
   bool allow_deleting_history =
       prefs->GetBoolean(prefs::kAllowDeletingBrowserHistory);
   source->AddBoolean("allowDeletingHistory", allow_deleting_history);
 
-  source->AddBoolean("showMenuPromo",
-      !prefs->GetBoolean(prefs::kMdHistoryMenuPromoShown));
+  source->AddBoolean(kShowMenuPromoKey, !MenuPromoShown(profile));
 
   bool group_by_domain = base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kHistoryEnableGroupByDomain) || profile->IsSupervised();
   source->AddBoolean("groupByDomain", group_by_domain);
 
   source->AddBoolean("isGuestSession", profile->IsGuestSession());
 
-  SigninManagerBase* signin_manager =
-      SigninManagerFactory::GetForProfile(profile);
-  bool is_authenticated = signin_manager != nullptr &&
-      signin_manager->IsAuthenticated();
-  source->AddBoolean("isUserSignedIn", is_authenticated);
+  source->AddBoolean(kIsUserSignedInKey, IsUserSignedIn(profile));
 
   source->AddResourcePath("constants.html", IDR_MD_HISTORY_CONSTANTS_HTML);
   source->AddResourcePath("constants.js", IDR_MD_HISTORY_CONSTANTS_JS);
   source->AddResourcePath("images/100/sign_in_promo.png",
                           IDR_MD_HISTORY_IMAGES_100_SIGN_IN_PROMO_PNG);
   source->AddResourcePath("images/200/sign_in_promo.png",
                           IDR_MD_HISTORY_IMAGES_200_SIGN_IN_PROMO_PNG);
   source->AddResourcePath("history.js", IDR_MD_HISTORY_HISTORY_JS);
 
 #if BUILDFLAG(USE_VULCANIZE)
@@ skipping 63 matching lines @@
   source->SetJsonPath("strings.js");
 
   return source;
 }
 
 }  // namespace
 
 bool MdHistoryUI::use_test_title_ = false;
 
 MdHistoryUI::MdHistoryUI(content::WebUI* web_ui) : WebUIController(web_ui) {
+  Profile* profile = Profile::FromWebUI(web_ui);
+  content::WebUIDataSource* data_source =
+      CreateMdHistoryUIHTMLSource(profile, use_test_title_);

[calamity, 2016/11/16 05:36:13]

Is there a reason we don't disable data source replacement here? Why do we need
a new data source per tab?

[Dan Beam, 2016/11/16 06:02:03]

so, right now these data sources live in the resource context on the IO thread.

WebUIs and associated code are created (and generally used from) the UI thread.

there's currently no retrieval API (and if there was, it might need to be
async?) so it'd be slightly hard to tell from most WebUI code whether an
existing data source is around (to not replace it).

fwiw: there is a TODO about this -
https://cs.chromium.org/chromium/src/content/public/browser/url_data_source.h...

we could attempt to just merge when Add() is called with an existing source. 
this would allow callers to retain references as the pointers to the objects
would stay the same, but be updated.

+  content::WebUIDataSource::Add(profile, data_source);
+
   web_ui->AddMessageHandler(new BrowsingHistoryHandler());
   web_ui->AddMessageHandler(new MetricsHandler());
 
   if (search::IsInstantExtendedAPIEnabled()) {
     web_ui->AddMessageHandler(new browser_sync::ForeignSessionHandler());
     web_ui->AddMessageHandler(new HistoryLoginHandler(
-        base::Bind(&MdHistoryUI::CreateDataSource, base::Unretained(this))));
+        base::Bind(&MdHistoryUI::UpdateDataSource, base::Unretained(this))));
   }
 
-  CreateDataSource();
-
   web_ui->RegisterMessageCallback("menuPromoShown",
       base::Bind(&MdHistoryUI::HandleMenuPromoShown, base::Unretained(this)));
 }
 
 MdHistoryUI::~MdHistoryUI() {}
 
 // static
 bool MdHistoryUI::IsEnabled(Profile* profile) {
   return base::FeatureList::IsEnabled(features::kMaterialDesignHistory) &&
          !base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ skipping 18 matching lines @@
   return ResourceBundle::GetSharedInstance().LoadDataResourceBytesForScale(
       IDR_HISTORY_FAVICON, scale_factor);
 }
 
 void MdHistoryUI::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   registry->RegisterBooleanPref(prefs::kMdHistoryMenuPromoShown, false,
       user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
 }
 
-// TODO(lshang): Change to not re-create data source every time after we use
-// unique_ptr instead of raw pointers for data source.
-void MdHistoryUI::CreateDataSource() {
+void MdHistoryUI::UpdateDataSource() {
+  CHECK(web_ui());
+
   Profile* profile = Profile::FromWebUI(web_ui());
-  content::WebUIDataSource* data_source =
-      CreateMdHistoryUIHTMLSource(profile, use_test_title_);
-  content::WebUIDataSource::Add(profile, data_source);
+
+  std::unique_ptr<base::DictionaryValue> update(new base::DictionaryValue);
+  update->SetBoolean(kIsUserSignedInKey, IsUserSignedIn(profile));
+  update->SetBoolean(kShowMenuPromoKey, !MenuPromoShown(profile));
+
+  content::WebUIDataSource::Update(profile, chrome::kChromeUIHistoryHost,
+                                   std::move(update));
 }
 
 void MdHistoryUI::HandleMenuPromoShown(const base::ListValue* args) {
   Profile::FromWebUI(web_ui())->GetPrefs()->SetBoolean(
       prefs::kMdHistoryMenuPromoShown, true);
-  CreateDataSource();
+  UpdateDataSource();
 }