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

BUG=659487

Review-Url: https://codereview.chromium.org/2475593002
Cr-Commit-Position: refs/heads/master@{#437451}

chrome/browser/ui/webui/history_ui.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/webui/history_ui.h"
 
 #include <string>
+#include <utility>
 
 #include "base/command_line.h"
 #include "base/memory/ref_counted_memory.h"
@@ -60,20 +61,23 @@ const char kIncognitoModeShortcut[] = "(Ctrl+Shift+N)";
 const char kIncognitoModeShortcut[] = "(Shift+Ctrl+N)";
 #endif
 
-content::WebUIDataSource* CreateHistoryUIHTMLSource(Profile* profile) {
-  PrefService* prefs = profile->GetPrefs();
+constexpr char kIsUserSignedInKey[] = "isUserSignedIn";
 
+bool IsSignedIn(Profile* profile) {
   // Check if the profile is authenticated.  Guest profiles or incognito
   // windows may not have a sign in manager, and are considered not
   // authenticated.
   SigninManagerBase* signin_manager =
       SigninManagerFactory::GetForProfile(profile);
-  bool is_authenticated = signin_manager != nullptr &&
-      signin_manager->IsAuthenticated();
+  return signin_manager && signin_manager->IsAuthenticated();
+}
+
+content::WebUIDataSource* CreateHistoryUIHTMLSource(Profile* profile) {
+  PrefService* prefs = profile->GetPrefs();
 
   content::WebUIDataSource* source =
       content::WebUIDataSource::Create(chrome::kChromeUIHistoryFrameHost);
-  source->AddBoolean("isUserSignedIn", is_authenticated);
+  source->AddBoolean(kIsUserSignedInKey, IsSignedIn(profile));
 #if !defined(OS_ANDROID)
   source->AddLocalizedString("collapseSessionMenuItemText",
       IDS_HISTORY_OTHER_SESSIONS_COLLAPSE_SESSION);
@@ -173,6 +177,10 @@ content::WebUIDataSource* CreateHistoryUIHTMLSource(Profile* profile) {
 }  // namespace
 
 HistoryUI::HistoryUI(content::WebUI* web_ui) : WebUIController(web_ui) {
+  // Set up the chrome://history-frame/ source.
+  Profile* profile = Profile::FromWebUI(web_ui);
+  content::WebUIDataSource::Add(profile, CreateHistoryUIHTMLSource(profile));
+
   web_ui->AddMessageHandler(new BrowsingHistoryHandler());
   web_ui->AddMessageHandler(new MetricsHandler());
 
@@ -181,7 +189,7 @@ HistoryUI::HistoryUI(content::WebUI* web_ui) : WebUIController(web_ui) {
   if (search::IsInstantExtendedAPIEnabled()) {
     web_ui->AddMessageHandler(new browser_sync::ForeignSessionHandler());
     web_ui->AddMessageHandler(new HistoryLoginHandler(
-        base::Bind(&HistoryUI::CreateDataSource, base::Unretained(this))));
+        base::Bind(&HistoryUI::UpdateDataSource, base::Unretained(this))));
   }
 #endif
 
@@ -193,14 +201,11 @@ HistoryUI::HistoryUI(content::WebUI* web_ui) : WebUIController(web_ui) {
   // This code should be removed as soon as the API is ready.
   GURL url = web_ui->GetWebContents()->GetVisibleURL();
   if (url.has_query() && url.query() == "reset_ofbh") {
-    Profile::FromWebUI(web_ui)->GetPrefs()->SetInteger(
+    profile->GetPrefs()->SetInteger(
         browsing_data::prefs::kClearBrowsingDataHistoryNoticeShownTimes, 0);
     browsing_data::testing::
         g_override_other_forms_of_browsing_history_query = true;
   }
-
-  // Set up the chrome://history-frame/ source.
-  CreateDataSource();
 }
 
 HistoryUI::~HistoryUI() {}
@@ -212,10 +217,11 @@ base::RefCountedMemory* HistoryUI::GetFaviconResourceBytes(
       LoadDataResourceBytesForScale(IDR_HISTORY_FAVICON, scale_factor);
 }
 
-// TODO(lshang): Change to not re-create data source every time after we use
-// unique_ptr instead of raw pointers for data source.
-void HistoryUI::CreateDataSource() {
+void HistoryUI::UpdateDataSource() {
+  CHECK(web_ui());
   Profile* profile = Profile::FromWebUI(web_ui());
-  content::WebUIDataSource* data_source = CreateHistoryUIHTMLSource(profile);
-  content::WebUIDataSource::Add(profile, data_source);
+  std::unique_ptr<base::DictionaryValue> update(new base::DictionaryValue);
+  update->SetBoolean(kIsUserSignedInKey, IsSignedIn(profile));
+  content::WebUIDataSource::Update(profile, chrome::kChromeUIHistoryFrameHost,
+                                   std::move(update));
 }

chrome/browser/ui/webui/history_ui.h

@@ -22,7 +22,7 @@ class HistoryUI : public content::WebUIController {
       ui::ScaleFactor scale_factor);
 
  private:
-  void CreateDataSource();
+  void UpdateDataSource();
 
   DISALLOW_COPY_AND_ASSIGN(HistoryUI);
 };

chrome/browser/ui/webui/md_history_ui.cc

@@ -7,6 +7,7 @@
 #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"
@@ -35,6 +36,19 @@
 
 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 =
@@ -117,8 +131,7 @@ content::WebUIDataSource* CreateMdHistoryUIHTMLSource(Profile* profile,
       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();
@@ -126,11 +139,7 @@ content::WebUIDataSource* CreateMdHistoryUIHTMLSource(Profile* profile,
 
   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);
@@ -214,17 +223,20 @@ content::WebUIDataSource* CreateMdHistoryUIHTMLSource(Profile* profile,
 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_);
+  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)));
 }
@@ -252,17 +264,21 @@ void MdHistoryUI::RegisterProfilePrefs(
       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();
 }

chrome/browser/ui/webui/md_history_ui.h

@@ -39,7 +39,7 @@ class MdHistoryUI : public content::WebUIController {
 
   static bool use_test_title_;
 
-  void CreateDataSource();
+  void UpdateDataSource();
 
   // Handler for the "menuPromoShown" message from the page. No arguments.
   void HandleMenuPromoShown(const base::ListValue* args);

content/browser/webui/url_data_manager.cc

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/lazy_instance.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/message_loop/message_loop.h"
@@ -46,6 +47,14 @@ static void AddDataSourceOnIOThread(
       data_source.get());
 }
 
+static void UpdateWebUIDataSourceOnIOThread(
+    ResourceContext* resource_context,
+    std::string source_name,
+    const base::DictionaryValue* update) {
+  GetURLDataManagerForResourceContext(resource_context)
+      ->UpdateWebUIDataSource(source_name, *update);
+}
+
 }  // namespace
 
 // static
@@ -67,6 +76,17 @@ void URLDataManager::AddDataSource(URLDataSourceImpl* source) {
                  make_scoped_refptr(source)));
 }
 
+void URLDataManager::UpdateWebUIDataSource(
+    const std::string& source_name,
+    std::unique_ptr<base::DictionaryValue> update) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&UpdateWebUIDataSourceOnIOThread,
+                 browser_context_->GetResourceContext(), source_name,
+                 base::Owned(update.release())));
+}
+
 // static
 void URLDataManager::DeleteDataSources() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -122,6 +142,14 @@ void URLDataManager::AddWebUIDataSource(BrowserContext* browser_context,
   GetFromBrowserContext(browser_context)->AddDataSource(impl);
 }
 
+void URLDataManager::UpdateWebUIDataSource(
+    BrowserContext* browser_context,
+    const std::string& source_name,
+    std::unique_ptr<base::DictionaryValue> update) {
+  GetFromBrowserContext(browser_context)
+      ->UpdateWebUIDataSource(source_name, std::move(update));
+}
+
 // static
 bool URLDataManager::IsScheduledForDeletion(
     const URLDataSourceImpl* data_source) {

content/browser/webui/url_data_manager.h

@@ -5,6 +5,7 @@
 #ifndef CONTENT_BROWSER_WEBUI_URL_DATA_MANAGER_H_
 #define CONTENT_BROWSER_WEBUI_URL_DATA_MANAGER_H_
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -13,6 +14,10 @@
 #include "base/supports_user_data.h"
 #include "content/common/content_export.h"
 
+namespace base {
+class DictionaryValue;
+}
+
 namespace content {
 class BrowserContext;
 class URLDataSource;
@@ -41,6 +46,9 @@ class CONTENT_EXPORT URLDataManager : public base::SupportsUserData::Data {
   // destructed in the same thread as they are constructed (the UI thread).
   void AddDataSource(URLDataSourceImpl* source);
 
+  void UpdateWebUIDataSource(const std::string& source_name,
+                             std::unique_ptr<base::DictionaryValue> update);
+
   // Deletes any data sources no longer referenced. This is normally invoked
   // for you, but can be invoked to force deletion (such as during shutdown).
   static void DeleteDataSources();
@@ -55,6 +63,12 @@ class CONTENT_EXPORT URLDataManager : public base::SupportsUserData::Data {
   static void AddWebUIDataSource(BrowserContext* browser_context,
                                  WebUIDataSource* source);
 
+  // Updates an existing WebUI data source.
+  static void UpdateWebUIDataSource(
+      BrowserContext* browser_context,
+      const std::string& source_name,
+      std::unique_ptr<base::DictionaryValue> update);
+
  private:
   friend class URLDataSourceImpl;
   friend struct DeleteURLDataSource;

content/browser/webui/url_data_manager_backend.cc

@@ -33,6 +33,7 @@
 #include "content/browser/resource_context_impl.h"
 #include "content/browser/webui/shared_resources_data_source.h"
 #include "content/browser/webui/url_data_source_impl.h"
+#include "content/browser/webui/web_ui_data_source_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
@@ -650,6 +651,18 @@ void URLDataManagerBackend::AddDataSource(
   source->backend_ = this;
 }
 
+void URLDataManagerBackend::UpdateWebUIDataSource(
+    const std::string& source_name,
+    const base::DictionaryValue& update) {
+  DataSourceMap::iterator it = data_sources_.find(source_name);
+  if (it == data_sources_.end() || !it->second->IsWebUIDataSourceImpl()) {
+    NOTREACHED();
+    return;
+  }
+  static_cast<WebUIDataSourceImpl*>(it->second.get())
+      ->AddLocalizedStrings(update);
+}
+
 bool URLDataManagerBackend::HasPendingJob(
     URLRequestChromeJob* job) const {
   for (PendingRequestMap::const_iterator i = pending_requests_.begin();

content/browser/webui/url_data_manager_backend.h

@@ -13,6 +13,7 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/supports_user_data.h"
+#include "base/values.h"
 #include "content/browser/webui/url_data_manager.h"
 #include "content/public/browser/url_data_source.h"
 #include "net/url_request/url_request_job_factory.h"
@@ -52,6 +53,9 @@ class URLDataManagerBackend : public base::SupportsUserData::Data {
   // Adds a DataSource to the collection of data sources.
   void AddDataSource(URLDataSourceImpl* source);
 
+  void UpdateWebUIDataSource(const std::string& source_name,
+                             const base::DictionaryValue& update);
+
   // DataSource invokes this. Sends the data to the URLRequest. |bytes| may be
   // null, which signals an error handling the request.
   void DataAvailable(RequestID request_id, base::RefCountedMemory* bytes);

content/browser/webui/url_data_source_impl.cc

@@ -45,6 +45,10 @@ void URLDataSourceImpl::SendResponse(
                                      this, request_id, std::move(bytes)));
 }
 
+bool URLDataSourceImpl::IsWebUIDataSourceImpl() const {
+  return false;
+}
+
 void URLDataSourceImpl::SendResponseOnIOThread(
     int request_id,
     scoped_refptr<base::RefCountedMemory> bytes) {

content/browser/webui/url_data_source_impl.h

@@ -63,6 +63,8 @@ class URLDataSourceImpl : public base::RefCountedThreadSafe<
   const std::string& source_name() const { return source_name_; }
   URLDataSource* source() const { return source_.get(); }
 
+  virtual bool IsWebUIDataSourceImpl() const;
+
  protected:
   virtual ~URLDataSourceImpl();
 

content/browser/webui/web_ui_data_source_impl.cc

@@ -35,6 +35,14 @@ void WebUIDataSource::Add(BrowserContext* browser_context,
   URLDataManager::AddWebUIDataSource(browser_context, source);
 }
 
+// static
+void WebUIDataSource::Update(BrowserContext* browser_context,
+                             const std::string& source_name,
+                             std::unique_ptr<base::DictionaryValue> update) {
+  URLDataManager::UpdateWebUIDataSource(browser_context, source_name,
+                                        std::move(update));
+}
+
 // Internal class to hide the fact that WebUIDataSourceImpl implements
 // URLDataSource.
 class WebUIDataSourceImpl::InternalDataSource : public URLDataSource {
@@ -182,6 +190,10 @@ void WebUIDataSourceImpl::ExcludePathFromGzip(const std::string& path) {
   excluded_paths_.insert(path);
 }
 
+bool WebUIDataSourceImpl::IsWebUIDataSourceImpl() const {
+  return true;
+}
+
 void WebUIDataSourceImpl::DisableContentSecurityPolicy() {
   add_csp_ = false;
 }

content/browser/webui/web_ui_data_source_impl.h

@@ -54,6 +54,8 @@ class CONTENT_EXPORT WebUIDataSourceImpl
   // When DisableI18nAndUseGzipForAllPaths is enabled, exclude the given |path|.
   void ExcludePathFromGzip(const std::string& path);
 
+  bool IsWebUIDataSourceImpl() const override;
+
  protected:
   ~WebUIDataSourceImpl() override;
 

content/public/browser/web_ui_data_source.h

@@ -7,6 +7,8 @@
 
 #include <stdint.h>
 
+#include <memory>
+
 #include "base/callback.h"
 #include "base/strings/string16.h"
 #include "content/common/content_export.h"
@@ -27,10 +29,18 @@ class WebUIDataSource {
 
   CONTENT_EXPORT static WebUIDataSource* Create(const std::string& source_name);
 
-  // Adds a WebUI data source to |browser_context|.
+  // Adds a WebUI data source to |browser_context|. TODO(dbeam): update this API
+  // to take a std::unique_ptr instead to make it clear that |source| can be
+  // destroyed and references should not be kept by callers. Use |Update()|
+  // if you need to change an existing data source.
   CONTENT_EXPORT static void Add(BrowserContext* browser_context,
                                  WebUIDataSource* source);
 
+  CONTENT_EXPORT static void Update(
+      BrowserContext* browser_context,
+      const std::string& source_name,
+      std::unique_ptr<base::DictionaryValue> update);
+
   // Adds a string keyed to its name to our dictionary.
   virtual void AddString(const std::string& name,
                          const base::string16& value) = 0;