non-new-profile-management creates a "no-op" style account_reconcilor,

components/signin/core/browser/account_reconcilor.cc

 // Copyright 2014 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 "components/signin/core/browser/account_reconcilor.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/time/time.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_client.h"
 #include "components/signin/core/browser/signin_oauth_helper.h"
+#include "components/signin/core/common/profile_management_switches.h"
 #include "google_apis/gaia/gaia_auth_fetcher.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "google_apis/gaia/gaia_constants.h"
 #include "google_apis/gaia/gaia_oauth_client.h"
 #include "google_apis/gaia/gaia_urls.h"
 #include "net/cookies/canonical_cookie.h"
 
 
 namespace {
 
@@ skipping 165 matching lines @@
       signin_manager_(signin_manager),
       client_(client),
       merge_session_helper_(token_service_,
                             client->GetURLRequestContext(),
                             this),
       registered_with_token_service_(false),
       is_reconcile_started_(false),
       are_gaia_accounts_set_(false),
       requests_(NULL) {
   VLOG(1) << "AccountReconcilor::AccountReconcilor";
+  noop_only_mode_ = !switches::IsNewProfileManagement();
 }
 
 AccountReconcilor::~AccountReconcilor() {
   VLOG(1) << "AccountReconcilor::~AccountReconcilor";
   // Make sure shutdown was called first.
   DCHECK(!registered_with_token_service_);
   DCHECK(!reconciliation_timer_.IsRunning());
   DCHECK(!requests_);
   DCHECK_EQ(0u, user_id_fetchers_.size());
   DCHECK_EQ(0u, refresh_token_fetchers_.size());
@@ skipping 118 matching lines @@
 
 void AccountReconcilor::OnCookieChanged(const net::CanonicalCookie* cookie) {
   if (cookie->Name() == "LSID" &&
       cookie->Domain() == GaiaUrls::GetInstance()->gaia_url().host() &&
       cookie->IsSecure() && cookie->IsHttpOnly()) {
     VLOG(1) << "AccountReconcilor::OnCookieChanged: LSID changed";
 #ifdef OS_CHROMEOS
     // On Chrome OS it is possible that O2RT is not available at this moment
     // because profile data transfer is still in progress.
     if (!token_service_->GetAccounts().size()) {
       VLOG(1) << "AccountReconcilor::OnCookieChanged: cookie change is ingored"

[Roger Tawa OOO till Jul 10th, 2014/05/07 16:14:45]

Could you fix this typo: ingored -> ignored

[Mike Lerman, 2014/05/08 15:17:50]

Done.

                  "because profile data transfer is in progress.";
       return;
     }
 #endif
     StartReconcile();
   }
 }
 
 void AccountReconcilor::OnRefreshTokenAvailable(const std::string& account_id) {
   VLOG(1) << "AccountReconcilor::OnRefreshTokenAvailable: " << account_id;
@@ skipping 16 matching lines @@
 }
 
 void AccountReconcilor::GoogleSignedOut(const std::string& username) {
   VLOG(1) << "AccountReconcilor::GoogleSignedOut: signed out";
   UnregisterWithTokenService();
   UnregisterForCookieChanges();
   StopPeriodicReconciliation();
 }
 
 void AccountReconcilor::PerformMergeAction(const std::string& account_id) {
+  if (noop_only_mode_)
+    return;
   VLOG(1) << "AccountReconcilor::PerformMergeAction: " << account_id;
   merge_session_helper_.LogIn(account_id);
 }
 
 void AccountReconcilor::StartRemoveAction(const std::string& account_id) {
   VLOG(1) << "AccountReconcilor::StartRemoveAction: " << account_id;
   GetAccountsFromCookie(base::Bind(&AccountReconcilor::FinishRemoveAction,
                                    base::Unretained(this),
                                    account_id));
 }
 
 void AccountReconcilor::FinishRemoveAction(
     const std::string& account_id,
     const GoogleServiceAuthError& error,
     const std::vector<std::pair<std::string, bool> >& accounts) {
   VLOG(1) << "AccountReconcilor::FinishRemoveAction:"
           << " account=" << account_id << " error=" << error.ToString();
   if (error.state() == GoogleServiceAuthError::NONE) {
     AbortReconcile();
     std::vector<std::string> accounts_only;
     for (std::vector<std::pair<std::string, bool> >::const_iterator i =
              accounts.begin();
          i != accounts.end();
          ++i) {
       accounts_only.push_back(i->first);
     }
     merge_session_helper_.LogOut(account_id, accounts_only);

[Roger Tawa OOO till Jul 10th, 2014/05/07 16:14:45]

Most calls to |merge_session_helper_| have side effects that should not happen
in no-op mode.  This is one of them.  We should make a check at the top of this
function.

We can also add a check in StartRemoveAction() to avoid an unnecessary round
trip to gaia in the call to GetAccountsFromCookie().

Also, to increase consistency, I wonder if we should rename these two function
so that "Perform" is in the name.  That way we know which methods have side
effects:

StartRemoveAction --> StartPerformRemoveAction
FinishRemoveAction --> FinishPerformRemoveAction

[Mike Lerman, 2014/05/08 15:17:50]

Functions renamed.
Add a check on top of PerformFinishRemoveAction.
As discussed, did not add a check to PerformStartRemoveAction because logging
information about the accounts fetch is interesting and desirable.

   }
   // Wait for the next ReconcileAction if there is an error.
 }
 
 void AccountReconcilor::PerformAddToChromeAction(const std::string& account_id,
                                                  int session_index) {
+  if (noop_only_mode_)
+    return;
   VLOG(1) << "AccountReconcilor::PerformAddToChromeAction:"
           << " account=" << account_id << " session_index=" << session_index;
 
 #if !defined(OS_ANDROID) && !defined(OS_IOS)
   refresh_token_fetchers_.push_back(
       new RefreshTokenFetcher(this, account_id, session_index));
 #endif
 }
 
 void AccountReconcilor::PerformLogoutAllAccountsAction() {
+  if (noop_only_mode_)
+    return;
   VLOG(1) << "AccountReconcilor::PerformLogoutAllAccountsAction";
   merge_session_helper_.LogOutAllAccounts();
 }
 
 void AccountReconcilor::StartReconcile() {
   if (!IsProfileConnected() || is_reconcile_started_)
     return;
 
   is_reconcile_started_ = true;
 
@@ skipping 288 matching lines @@
 
 void AccountReconcilor::HandleFailedAccountIdCheck(
     const std::string& account_id) {
   invalid_chrome_accounts_.insert(account_id);
   FinishReconcile();
 }
 
 void AccountReconcilor::HandleRefreshTokenFetched(
     const std::string& account_id,
     const std::string& refresh_token) {
   if (!refresh_token.empty()) {

[Roger Tawa OOO till Jul 10th, 2014/05/07 16:14:45]

Should probably also add a check to this if block too, since it could change the
state of the token service.  In theory we should never get here since the check
in PerformAddToChromeAction() blocks the series of actions that would eventually
call HandleRefreshTokenFetched(), but might be safe.

Alternatively, it might also be more consistent with the rest of the code to
move the one line 739 into a new function like
PerformAddAccountToTokenService(account_id, refresh_token) and add the check for
the noop flag in there.  What do you think?

[Mike Lerman, 2014/05/08 15:17:50]

Added the check, you're right it doesn't hurt (much).
This was implemented in a new PerformAddAccountToTokenService method to maintain
the pattern of Perform* methods checking the flag.

     token_service_->UpdateCredentials(account_id, refresh_token);
   }
 
   // Remove the account from the list that is being updated.
   for (std::vector<std::pair<std::string, int> >::iterator i =
            add_to_chrome_.begin();
        i != add_to_chrome_.end();
        ++i) {
     if (gaia::AreEmailsSame(account_id, i->first)) {
       add_to_chrome_.erase(i);
       break;
     }
   }
 
   CalculateIfReconcileIsDone();
 }