MD History: Update sign in state in data source

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 "build/build_config.h"
@@ skipping 197 matching lines @@
   source->SetDefaultResource(IDR_MD_HISTORY_HISTORY_HTML);
   source->SetJsonPath("strings.js");
 
   return source;
 }
 
 }  // namespace
 
 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.

   content::WebUIDataSource::Add(profile, data_source_);
 
   web_ui->RegisterMessageCallback("menuPromoShown",
       base::Bind(&MdHistoryUI::HandleMenuPromoShown, base::Unretained(this)));
 }
 
 MdHistoryUI::~MdHistoryUI() {}
 
 // static
 bool MdHistoryUI::IsEnabled(Profile* profile) {
@@ skipping 23 matching lines @@
 
 void MdHistoryUI::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   registry->RegisterBooleanPref(prefs::kMdHistoryMenuPromoShown, false,
       user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
 }
 
 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.

+}