MD History: Update sign in state in data source

chrome/browser/ui/webui/md_history_ui.cc

Index: chrome/browser/ui/webui/md_history_ui.cc
diff --git a/chrome/browser/ui/webui/md_history_ui.cc b/chrome/browser/ui/webui/md_history_ui.cc
index 56bbf9b69003e58ce5d8a0fa4a19f3f7d834920a..b15b4ed2bfdd9a221ffea2baac2018716b2e0f75 100644
--- a/chrome/browser/ui/webui/md_history_ui.cc
+++ b/chrome/browser/ui/webui/md_history_ui.cc
@@ -215,14 +215,16 @@ content::WebUIDataSource* CreateMdHistoryUIHTMLSource(Profile* profile,
 
 bool MdHistoryUI::use_test_title_ = false;
 
-MdHistoryUI::MdHistoryUI(content::WebUI* web_ui) : WebUIController(web_ui) {
+MdHistoryUI::MdHistoryUI(content::WebUI* web_ui)
+    : WebUIController(web_ui), weak_ptr_factory_(this) {
   Profile* profile = Profile::FromWebUI(web_ui);
   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());
+    web_ui->AddMessageHandler(new HistoryLoginHandler(base::Bind(
+        &MdHistoryUI::SignInChanged, weak_ptr_factory_.GetWeakPtr())));

[Dan Beam, 2016/09/29 18:13:33]

you can use weakptr, but now that we're not dependent on the datasource's
lifecycle i'm fairly sure base::Unretained() will work fine as each handler is
owned by a webui (i.e. this class owns the HistoryLoginHandler)

[lshang, 2016/09/30 00:33:19]

Done.

   }
 
   data_source_ = CreateMdHistoryUIHTMLSource(profile, use_test_title_);

[Dan Beam, 2016/09/29 18:13:33]

don't keep this pointer around, as it's potentially garbage if it's been
overwritten (which can lead to crashes)

[lshang, 2016/09/30 00:33:19]

Done. Added a Get() method to create the data source so that the data source
pointer it returns are always valid.

@@ -269,5 +271,15 @@ void MdHistoryUI::RegisterProfilePrefs(
 void MdHistoryUI::HandleMenuPromoShown(const base::ListValue* args) {
   Profile::FromWebUI(web_ui())->GetPrefs()->SetBoolean(
       prefs::kMdHistoryMenuPromoShown, true);
+  Profile* profile = Profile::FromWebUI(web_ui());
+  data_source_ = CreateMdHistoryUIHTMLSource(profile, use_test_title_);
+  content::WebUIDataSource::Add(profile, data_source_);

[lshang, 2016/09/29 08:06:48]

I also modified here due to the same reason as below. Crash is seen when:

1. open one tab(not showing the promo)
2. open another tab, narrow the screent to make the promo shown
3. go back to the first tab **without** reloading, make the promo shown as well

They are all because WebUIDataSource gets destroyed and replaced between
multiple tabs.

   data_source_->AddBoolean(kShowMenuPromoKey, false);
 }
+
+void MdHistoryUI::SignInChanged(bool signed_in) {
+  // Recreate the data source so sign in state get updated in there.
+  Profile* profile = Profile::FromWebUI(web_ui());
+  data_source_ = CreateMdHistoryUIHTMLSource(profile, use_test_title_);
+  content::WebUIDataSource::Add(profile, data_source_);

[lshang, 2016/09/29 08:06:48]

I found that purely AddBoolean() here will cause browser crashes when:

1. Open a tab, go to chrome://history
2. Open another tab, sign in/out

I debugged it a bit. When opening a new tab, the browser will create a new
WebUIDataSource, and this data source will replace first tab's data source, so
the first one is destroyed. But when this callback method get runned, it still
try to call the already-been-replaced data source and segmentation fault occurs.

My solution here is re-create the data source of the current tab, so we don't
need to do AddBoolean() here again in this way.

[Dan Beam, 2016/09/29 18:13:33]

wow, that's crazy sauce.  we should just make handlers not nuke eachothers
sources or add a Get() method to get the current data source

[lshang, 2016/09/30 00:33:19]

Done.

+}