MD History: Update sign in state in data source

chrome/browser/ui/webui/history_ui.cc

 // Copyright (c) 2012 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/history_ui.h"
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/memory/ref_counted_memory.h"
@@ skipping 42 matching lines @@
 const char kIncognitoModeShortcut[] = "("
     "\xE2\x87\xA7"  // Shift symbol (U+21E7 'UPWARDS WHITE ARROW').
     "\xE2\x8C\x98"  // Command symbol (U+2318 'PLACE OF INTEREST SIGN').
     "N)";
 #elif defined(OS_WIN)
 const char kIncognitoModeShortcut[] = "(Ctrl+Shift+N)";
 #else
 const char kIncognitoModeShortcut[] = "(Shift+Ctrl+N)";
 #endif
 
 content::WebUIDataSource* CreateHistoryUIHTMLSource(Profile* profile) {

[Dan Beam, 2016/09/30 00:47:43]

is there a reason why this is an anonymous method rather than an instance
method?  if this was an instance method, it'd have access to web_ui() and hence
able to get the profile

[lshang, 2016/09/30 01:31:08]

Probably in order to enclose this function inside this file? 

There is also a saying that anonymous namespace can improve performance since
symbols within the namespace do not need any external linkage and the compiler
is freer to perform aggressive optimization of the code within the namespace.
ExtensionsUI, MdDownloadsUI use it as anonymous as well. 

I'm not sure which design is better, but I'm happy to change  it if you feel
strongly about it:-)

   PrefService* prefs = profile->GetPrefs();
 
   // 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();
 
@@ skipping 95 matching lines @@
 
   return source;
 }
 
 }  // namespace
 
 HistoryUI::HistoryUI(content::WebUI* web_ui) : WebUIController(web_ui) {
   web_ui->AddMessageHandler(new BrowsingHistoryHandler());
   web_ui->AddMessageHandler(new MetricsHandler());
 
+  // Set up the chrome://history-frame/ source.
+  CreateAndGetDataSource();
+
   // On mobile we deal with foreign sessions differently.
 #if !defined(OS_ANDROID)
   if (search::IsInstantExtendedAPIEnabled()) {
     web_ui->AddMessageHandler(new browser_sync::ForeignSessionHandler());
-    web_ui->AddMessageHandler(new HistoryLoginHandler());
+    web_ui->AddMessageHandler(new HistoryLoginHandler(
+        base::Bind(&HistoryUI::SignInChanged, base::Unretained(this))));
   }
 #endif
 
   // TODO(crbug.com/595332): Since the API to query other forms of browsing
   // history is not ready yet, make it possible to test the history UI as if
   // it were. If the user opens chrome://history/?reset_ofbh, we will assume
   // that other forms of browsing history exist (for all accounts), and we will
   // also reset the one-time notice shown in the Clear Browsing Data dialog.
   // 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(
         browsing_data::prefs::kClearBrowsingDataHistoryNoticeShownTimes, 0);
     browsing_data::testing::
         g_override_other_forms_of_browsing_history_query = true;
   }
-
-  // Set up the chrome://history-frame/ source.
-  Profile* profile = Profile::FromWebUI(web_ui);
-  content::WebUIDataSource::Add(profile, CreateHistoryUIHTMLSource(profile));
 }
 
 HistoryUI::~HistoryUI() {}
 
 // static
 base::RefCountedMemory* HistoryUI::GetFaviconResourceBytes(
       ui::ScaleFactor scale_factor) {
   return ResourceBundle::GetSharedInstance().
       LoadDataResourceBytesForScale(IDR_HISTORY_FAVICON, scale_factor);
 }
+
+content::WebUIDataSource* HistoryUI::CreateAndGetDataSource() {
+  Profile* profile = Profile::FromWebUI(web_ui());
+  content::WebUIDataSource* data_source = CreateHistoryUIHTMLSource(profile);
+  content::WebUIDataSource::Add(profile, data_source);
+  return data_source;

[Dan Beam, 2016/09/30 00:47:43]

so, groby@ is going to make this API take a std::unique_ptr to make it clear
that this method takes ownership of this memory and that we shouldn't keep raw
pointers around if they're gonna be deleted from under us.  so don't return the
pointer from this method.  also, I don't see where the return value is used in
this method anyways...

[lshang, 2016/09/30 01:31:08]

Changed to just create it. groby@'s change would be great to avoid crashes,
should I wait for her CL to land first?

[Dan Beam, 2016/09/30 02:55:40]

let's just re-create every time with a TODO for now

[lshang, 2016/09/30 03:21:13]

Done.

+}
+
+void HistoryUI::SignInChanged(bool signed_in) {
+  // Recreate the data source so sign in state get updated in there.
+  CreateAndGetDataSource();
+}