MD History: Update sign in state in data source

chrome/browser/ui/webui/history_login_handler.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/history_login_handler.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/values.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/webui/profile_info_watcher.h"
 #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() {}
+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() {}
 
 void HistoryLoginHandler::RegisterMessages() {
   profile_info_watcher_.reset(new ProfileInfoWatcher(
       Profile::FromWebUI(web_ui()),
       base::Bind(&HistoryLoginHandler::ProfileInfoChanged,
                  base::Unretained(this))));
 
   web_ui()->RegisterMessageCallback("otherDevicesInitialized",
       base::Bind(&HistoryLoginHandler::HandleOtherDevicesInitialized,
                  base::Unretained(this)));
 
   web_ui()->RegisterMessageCallback("startSignInFlow",
       base::Bind(&HistoryLoginHandler::HandleStartSignInFlow,
                  base::Unretained(this)));
 }
 
 void HistoryLoginHandler::HandleOtherDevicesInitialized(
     const base::ListValue* /*args*/) {
   ProfileInfoChanged();
 }
 
 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(
     const base::ListValue* /*args*/) {
   Browser* browser =
       chrome::FindBrowserWithWebContents(web_ui()->GetWebContents());
   browser->window()->ShowAvatarBubbleFromAvatarButton(
       BrowserWindow::AVATAR_BUBBLE_MODE_SIGNIN,
       signin::ManageAccountsParams(),
       signin_metrics::AccessPoint::ACCESS_POINT_RECENT_TABS);
 }