📰 Let the backend trigger sign in related refreshes

components/ntp_snippets/remote/ntp_snippets_status_service.cc

Index: components/ntp_snippets/remote/ntp_snippets_status_service.cc
diff --git a/components/ntp_snippets/remote/ntp_snippets_status_service.cc b/components/ntp_snippets/remote/ntp_snippets_status_service.cc
index 62514d270b101cc714a8169744ae08f4d88ccec1..7cf74db1e1c08b16562387f9b31dc4ea3f4ade38 100644
--- a/components/ntp_snippets/remote/ntp_snippets_status_service.cc
+++ b/components/ntp_snippets/remote/ntp_snippets_status_service.cc
@@ -10,6 +10,7 @@
 #include "components/ntp_snippets/pref_names.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
+#include "components/signin/core/browser/signin_manager.h"
 #include "components/variations/variations_associated_data.h"
 
 namespace ntp_snippets {
@@ -28,8 +29,7 @@ NTPSnippetsStatusService::NTPSnippetsStatusService(
     : snippets_status_(SnippetsStatus::EXPLICITLY_DISABLED),
       require_signin_(false),
       signin_manager_(signin_manager),
-      pref_service_(pref_service),
-      signin_observer_(this) {
+      pref_service_(pref_service) {
   std::string param_value_str = variations::GetVariationParamValueByFeature(
       kArticleSuggestionsFeature, kFetchingRequiresSignin);
   if (param_value_str == kFetchingRequiresSigninEnabled) {
@@ -61,8 +61,6 @@ void NTPSnippetsStatusService::Init(
   snippets_status_ = GetSnippetsStatusFromDeps();
   snippets_status_change_callback_.Run(old_snippets_status, snippets_status_);
 
-  signin_observer_.Add(signin_manager_);
-
   pref_change_registrar_.Init(pref_service_);
   pref_change_registrar_.Add(
       prefs::kEnableSnippets,
@@ -87,15 +85,7 @@ bool NTPSnippetsStatusService::IsSignedIn() const {
   return signin_manager_ && signin_manager_->IsAuthenticated();

[Marc Treib, 2016/11/21 15:48:53]

Hm... a bit weird that this still needs the SigninManager, but relies on
external notifications that something changed. Could we add a "bool signed_in"
to OnSignInStateChanged and get rid of SigninManager here?

[dgn, 2016/11/21 16:22:08]

there are other cases where it need to pull the information, because it's
observing other things, for example preference changes. If that notification
comes before the signin state one, we might get into a bad state. Also, we don't
receive state notifications until after we ran the callback once, to properly
initialise the state. (Init() method.)

So because that bit might be delicate, I preferred staying as close as possible
to the previous implementation. That is a change that can be made independently
of the rest of the patch.

[Marc Treib, 2016/11/21 16:52:30]

Okay, fair enough. Could you add a TODO though?

 }
 
-void NTPSnippetsStatusService::GoogleSigninSucceeded(
-    const std::string& account_id,
-    const std::string& username,
-    const std::string& password) {
-  OnStateChanged(GetSnippetsStatusFromDeps());
-}
-
-void NTPSnippetsStatusService::GoogleSignedOut(const std::string& account_id,
-                                               const std::string& username) {
+void NTPSnippetsStatusService::OnSignInStateChanged() {
   OnStateChanged(GetSnippetsStatusFromDeps());
 }