MD History: Update sign in state in data source

chrome/browser/ui/webui/history_login_handler.cc

Index: chrome/browser/ui/webui/history_login_handler.cc
diff --git a/chrome/browser/ui/webui/history_login_handler.cc b/chrome/browser/ui/webui/history_login_handler.cc
index 1e4e89d8dd03d399d5f16ce404fc957fc120fec0..50d799fdc7e45d51356492241e3a7afd9c12f642 100644
--- a/chrome/browser/ui/webui/history_login_handler.cc
+++ b/chrome/browser/ui/webui/history_login_handler.cc
@@ -15,8 +15,11 @@
 #include "components/signin/core/browser/signin_metrics.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
+#include "content/public/browser/web_ui_data_source.h"
+
+HistoryLoginHandler::HistoryLoginHandler(content::WebUIDataSource* data_source)

[Dan Beam, 2016/09/28 03:56:49]

could we just pass a base::Callback instead of a WebUIDataSource*?

i.e.

#include "base/callback_forward.h"

class HistoryLoginHandler {
 private:
  base::Callback<void(bool)> signin_callback_;
};

#include "base/callback.h"

HistoryLoginHandler::HistoryLoginHandler(
    const base::Callback<void(bool)>& signin_callback)
    : signin_callback_(signin_callback) {}

void HistoryLoginHandler::ProfileInfoChanged() {
  bool signed_in = !profile_info_watcher_->GetAuthenticatedUsername().empty();
  if (!signin_callback_.is_null())
    signin_callback_.Run(signed_in);
}

and each HistoryUI creates a handler like this:

  HistoryUI::HistoryUI(content::WebUI* web_ui) {
    web_ui->AddMessageHandler(new HistoryLoginHandler(
        base::Bind(&HistoryUI::SignInChanged, base::Unretained(this)));
    // or use a WeakPtr if you're not sure about lifetime issues.
  }

  void HistoryUI::SignInChanged(bool signed_in) {
    data_source_->AddBoolean(kUserSignedInKey, signed_in);
  }

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

Done.

+    : data_source_(data_source) {}
 
-HistoryLoginHandler::HistoryLoginHandler() {}
 HistoryLoginHandler::~HistoryLoginHandler() {}
 
 void HistoryLoginHandler::RegisterMessages() {
@@ -41,8 +44,11 @@ void HistoryLoginHandler::HandleOtherDevicesInitialized(
 
 void HistoryLoginHandler::ProfileInfoChanged() {
   bool signed_in = !profile_info_watcher_->GetAuthenticatedUsername().empty();
-  web_ui()->CallJavascriptFunctionUnsafe("updateSignInState",
-                                         base::FundamentalValue(signed_in));
+  data_source_->AddBoolean("isUserSignedIn", signed_in);

[Dan Beam, 2016/09/28 03:56:49]

this also duplicates the name of this key (not so great) and provides access to
the data source (preferable to not do if possible)

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

NaN after we re-create the data source here and this name is not needed here.

+  AllowJavascript();

[Dan Beam, 2016/09/27 23:16:23]

you only want to call AllowJavascript() when the page tells the handler it's
ready.  how is that guaranteed here?  I think the profile info could change
before the page is ready or while it's reloading (which means it might be
ignored or be run on the wrong URL [i.e. any domain] or crash chrome)

[calamity, 2016/09/28 02:54:23]

Hmm, the other handlers in history don't have initialization callbacks either.
In fact, that's probably why we get those random 'setForeignSessions is
undefined' errors on load.

+  CallJavascriptFunction("cr.webUIListenerCallback",
+                         base::StringValue("sign-in-state-updated"),
+                         base::FundamentalValue(signed_in));
 }
 
 void HistoryLoginHandler::HandleStartSignInFlow(