diff options
author | Allan Sandfeld Jensen <allan.jensen@theqtcompany.com> | 2016-08-01 12:59:39 +0200 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2016-08-04 12:40:43 +0000 |
commit | 28b1110370900897ab652cb420c371fab8857ad4 (patch) | |
tree | 41b32127d23b0df4f2add2a27e12dc87bddb260e /chromium/components/signin | |
parent | 399c965b6064c440ddcf4015f5f8e9d131c7a0a6 (diff) | |
download | qtwebengine-chromium-28b1110370900897ab652cb420c371fab8857ad4.tar.gz |
BASELINE: Update Chromium to 53.0.2785.41
Also adds a few extra files for extensions.
Change-Id: Iccdd55d98660903331cf8b7b29188da781830af4
Reviewed-by: Michael BrĂ¼ning <michael.bruning@qt.io>
Diffstat (limited to 'chromium/components/signin')
33 files changed, 1241 insertions, 117 deletions
diff --git a/chromium/components/signin/DEPS b/chromium/components/signin/DEPS index 89ffc6df341..7a268e8c84a 100644 --- a/chromium/components/signin/DEPS +++ b/chromium/components/signin/DEPS @@ -12,3 +12,10 @@ include_rules = [ "+net", "+sql", ] + +# Tests can use syncable_prefs +specific_include_rules = { + ".*unittest\.cc": [ + "+components/syncable_prefs", + ] +} diff --git a/chromium/components/signin/core/browser/BUILD.gn b/chromium/components/signin/core/browser/BUILD.gn index 32c6353640e..0c3eb74c1ff 100644 --- a/chromium/components/signin/core/browser/BUILD.gn +++ b/chromium/components/signin/core/browser/BUILD.gn @@ -16,6 +16,8 @@ source_set("browser") { "account_info.h", "account_info_fetcher.cc", "account_info_fetcher.h", + "account_investigator.cc", + "account_investigator.h", "account_reconcilor.cc", "account_reconcilor.h", "account_tracker_service.cc", @@ -118,6 +120,8 @@ source_set("test_support") { "fake_account_fetcher_service.h", "fake_auth_status_provider.cc", "fake_auth_status_provider.h", + "fake_gaia_cookie_manager_service.cc", + "fake_gaia_cookie_manager_service.h", "fake_profile_oauth2_token_service.cc", "fake_profile_oauth2_token_service.h", "fake_signin_manager.cc", @@ -141,6 +145,7 @@ source_set("unit_tests") { testonly = true sources = [ "account_info_unittest.cc", + "account_investigator_unittest.cc", "account_tracker_service_unittest.cc", "gaia_cookie_manager_service_unittest.cc", "refresh_token_annotation_request_unittest.cc", @@ -152,13 +157,17 @@ source_set("unit_tests") { deps = [ ":test_support", - "//components/os_crypt:os_crypt", + "//components/os_crypt:test_support", "//components/signin/core/common", + "//components/syncable_prefs:test_support", "//testing/gmock", ] if (is_chromeos) { - sources -= [ "signin_status_metrics_provider_unittest.cc" ] + sources -= [ + "account_investigator_unittest.cc", + "signin_status_metrics_provider_unittest.cc", + ] } } diff --git a/chromium/components/signin/core/browser/about_signin_internals.cc b/chromium/components/signin/core/browser/about_signin_internals.cc index 9ec0228cc17..e1ac32ceb97 100644 --- a/chromium/components/signin/core/browser/about_signin_internals.cc +++ b/chromium/components/signin/core/browser/about_signin_internals.cc @@ -6,6 +6,8 @@ #include <stddef.h> +#include <utility> + #include "base/command_line.h" #include "base/hash.h" #include "base/i18n/time_formatting.h" @@ -42,7 +44,7 @@ base::ListValue* AddSection(base::ListValue* parent_list, section->SetString("title", title); section->Set("data", section_contents); - parent_list->Append(section.release()); + parent_list->Append(std::move(section)); return section_contents; } @@ -54,7 +56,7 @@ void AddSectionEntry(base::ListValue* section_list, entry->SetString("label", field_name); entry->SetString("status", field_status); entry->SetString("time", field_time); - section_list->Append(entry.release()); + section_list->Append(std::move(entry)); } void AddCookieEntry(base::ListValue* accounts_list, @@ -65,7 +67,7 @@ void AddCookieEntry(base::ListValue* accounts_list, entry->SetString("email", field_email); entry->SetString("gaia_id", field_gaia_id); entry->SetString("valid", field_valid); - accounts_list->Append(entry.release()); + accounts_list->Append(std::move(entry)); } std::string SigninStatusFieldToLabel(UntimedSigninStatusField field) { @@ -374,6 +376,7 @@ void AboutSigninInternals::GoogleSignedOut(const std::string& account_id, void AboutSigninInternals::OnGaiaAccountsInCookieUpdated( const std::vector<gaia::ListedAccount>& gaia_accounts, + const std::vector<gaia::ListedAccount>& signed_out_account, const GoogleServiceAuthError& error) { if (error.state() != GoogleServiceAuthError::NONE) return; @@ -661,15 +664,16 @@ AboutSigninInternals::SigninStatus::ToValue( token_service->GetAccounts(); if(accounts_in_token_service.size() == 0) { - base::DictionaryValue* no_token_entry = new base::DictionaryValue(); + std::unique_ptr<base::DictionaryValue> no_token_entry( + new base::DictionaryValue()); no_token_entry->SetString("accountId", "No token in Token Service."); - account_info->Append(no_token_entry); + account_info->Append(std::move(no_token_entry)); } for(const std::string& account_id : accounts_in_token_service) { - base::DictionaryValue* entry = new base::DictionaryValue(); + std::unique_ptr<base::DictionaryValue> entry(new base::DictionaryValue()); entry->SetString("accountId", account_id); - account_info->Append(entry); + account_info->Append(std::move(entry)); } return signin_status; diff --git a/chromium/components/signin/core/browser/about_signin_internals.h b/chromium/components/signin/core/browser/about_signin_internals.h index 43cd96079cd..9d77dd61b87 100644 --- a/chromium/components/signin/core/browser/about_signin_internals.h +++ b/chromium/components/signin/core/browser/about_signin_internals.h @@ -99,6 +99,7 @@ class AboutSigninInternals // GaiaCookieManagerService::Observer implementations. void OnGaiaAccountsInCookieUpdated( const std::vector<gaia::ListedAccount>& gaia_accounts, + const std::vector<gaia::ListedAccount>& signed_out_accounts, const GoogleServiceAuthError& error) override; private: diff --git a/chromium/components/signin/core/browser/account_investigator.cc b/chromium/components/signin/core/browser/account_investigator.cc new file mode 100644 index 00000000000..03d72c9200c --- /dev/null +++ b/chromium/components/signin/core/browser/account_investigator.cc @@ -0,0 +1,261 @@ +// Copyright 2016 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_investigator.h" + +#include <algorithm> +#include <iterator> + +#include "base/base64.h" +#include "base/logging.h" +#include "base/sha1.h" +#include "base/time/time.h" +#include "components/prefs/pref_registry_simple.h" +#include "components/prefs/pref_service.h" +#include "components/signin/core/browser/signin_manager_base.h" +#include "components/signin/core/browser/signin_metrics.h" +#include "components/signin/core/common/signin_pref_names.h" +#include "google_apis/gaia/gaia_auth_util.h" +#include "google_apis/gaia/google_service_auth_error.h" + +using base::Time; +using base::TimeDelta; +using gaia::ListedAccount; +using signin_metrics::AccountRelation; +using signin_metrics::ReportingType; + +namespace { + +// Prefixed used when calculating cookie jar hash to differentiate between +// signed in and signed out accounts. +const char kSignedInHashPrefix[] = "i"; +const char kSignedOutHashPrefix[] = "o"; + +bool AreSame(const AccountInfo& info, const ListedAccount& account) { + return info.account_id == account.id; +} + +} // namespace + +const TimeDelta AccountInvestigator::kPeriodicReportingInterval = + TimeDelta::FromDays(1); + +AccountInvestigator::AccountInvestigator( + GaiaCookieManagerService* cookie_service, + PrefService* pref_service, + SigninManagerBase* signin_manager) + : cookie_service_(cookie_service), + pref_service_(pref_service), + signin_manager_(signin_manager) {} + +AccountInvestigator::~AccountInvestigator() {} + +// static +void AccountInvestigator::RegisterPrefs(PrefRegistrySimple* registry) { + registry->RegisterStringPref(prefs::kGaiaCookieHash, ""); + registry->RegisterDoublePref(prefs::kGaiaCookieChangedTime, 0); + registry->RegisterDoublePref(prefs::kGaiaCookiePeriodicReportTime, 0); +} + +void AccountInvestigator::Initialize() { + cookie_service_->AddObserver(this); + previously_authenticated_ = signin_manager_->IsAuthenticated(); + + Time previous = Time::FromDoubleT( + pref_service_->GetDouble(prefs::kGaiaCookiePeriodicReportTime)); + if (previous.is_null()) + previous = Time::Now(); + const TimeDelta delay = + CalculatePeriodicDelay(previous, Time::Now(), kPeriodicReportingInterval); + timer_.Start(FROM_HERE, delay, this, &AccountInvestigator::TryPeriodicReport); +} + +void AccountInvestigator::Shutdown() { + cookie_service_->RemoveObserver(this); + timer_.Stop(); +} + +void AccountInvestigator::OnAddAccountToCookieCompleted( + const std::string& account_id, + const GoogleServiceAuthError& error) { + // This hook isn't particularly useful for us. Most cookie jar changes fly by + // without invoking this method, and some sign ins cause this method can get + // called serveral times. +} + +void AccountInvestigator::OnGaiaAccountsInCookieUpdated( + const std::vector<ListedAccount>& signed_in_accounts, + const std::vector<ListedAccount>& signed_out_accounts, + const GoogleServiceAuthError& error) { + if (error != GoogleServiceAuthError::AuthErrorNone()) { + // If we are pending periodic reporting, leave the flag set, and we will + // continue next time the ListAccounts call succeeds. + return; + } + + // Handling this is tricky. We could be here because there was a change. We + // could be here because we tried to do periodic reporting but there wasn't + // a valid cached ListAccounts response ready for us. Or even both of these + // could be simultaneously happening, although this should be extremely + // infrequent. + const std::string old_hash(pref_service_->GetString(prefs::kGaiaCookieHash)); + const std::string new_hash( + HashAccounts(signed_in_accounts, signed_out_accounts)); + const bool currently_authenticated = signin_manager_->IsAuthenticated(); + if (old_hash != new_hash) { + SharedCookieJarReport(signed_in_accounts, signed_out_accounts, Time::Now(), + ReportingType::ON_CHANGE); + pref_service_->SetString(prefs::kGaiaCookieHash, new_hash); + pref_service_->SetDouble(prefs::kGaiaCookieChangedTime, + Time::Now().ToDoubleT()); + } else if (currently_authenticated && !previously_authenticated_) { + SignedInAccountRelationReport(signed_in_accounts, signed_out_accounts, + ReportingType::ON_CHANGE); + } + + // Handling periodic after on change means that if both report, there had to + // be a change, which means we will report a stable age of 0. This also + // guarantees that on a fresh install we always have a cookie changed pref. + if (periodic_pending_) { + DoPeriodicReport(signed_in_accounts, signed_out_accounts); + } + + previously_authenticated_ = currently_authenticated; +} + +// static +TimeDelta AccountInvestigator::CalculatePeriodicDelay(Time previous, + Time now, + TimeDelta interval) { + // Don't allow negatives incase previous is in the future. + const TimeDelta age = std::max(now - previous, TimeDelta()); + // Don't allow negative intervals for very old things. + return std::max(interval - age, TimeDelta()); +} + +// static +std::string AccountInvestigator::HashAccounts( + const std::vector<ListedAccount>& signed_in_accounts, + const std::vector<ListedAccount>& signed_out_accounts) { + std::vector<std::string> sorted_ids(signed_in_accounts.size()); + std::transform(std::begin(signed_in_accounts), std::end(signed_in_accounts), + std::back_inserter(sorted_ids), + [](const ListedAccount& account) { + return std::string(kSignedInHashPrefix) + account.id; + }); + std::transform(std::begin(signed_out_accounts), std::end(signed_out_accounts), + std::back_inserter(sorted_ids), + [](const ListedAccount& account) { + return std::string(kSignedOutHashPrefix) + account.id; + }); + std::sort(sorted_ids.begin(), sorted_ids.end()); + std::ostringstream stream; + std::copy(sorted_ids.begin(), sorted_ids.end(), + std::ostream_iterator<std::string>(stream)); + + // PrefService will slightly mangle some undisplayable characters, by encoding + // in Base64 we are sure to have all safe characters that PrefService likes. + std::string encoded; + base::Base64Encode(base::SHA1HashString(stream.str()), &encoded); + return encoded; +} + +// static +AccountRelation AccountInvestigator::DiscernRelation( + const AccountInfo& info, + const std::vector<ListedAccount>& signed_in_accounts, + const std::vector<ListedAccount>& signed_out_accounts) { + if (signed_in_accounts.empty() && signed_out_accounts.empty()) { + return AccountRelation::EMPTY_COOKIE_JAR; + } + auto signed_in_match_iter = std::find_if( + signed_in_accounts.begin(), signed_in_accounts.end(), + [&info](const ListedAccount& account) { return AreSame(info, account); }); + auto signed_out_match_iter = std::find_if( + signed_out_accounts.begin(), signed_out_accounts.end(), + [&info](const ListedAccount& account) { return AreSame(info, account); }); + if (signed_in_match_iter != signed_in_accounts.end()) { + if (signed_in_accounts.size() == 1) { + return signed_out_accounts.empty() + ? AccountRelation::SINGLE_SIGNED_IN_MATCH_NO_SIGNED_OUT + : AccountRelation::SINGLE_SINGED_IN_MATCH_WITH_SIGNED_OUT; + } else { + return AccountRelation::ONE_OF_SIGNED_IN_MATCH_ANY_SIGNED_OUT; + } + } else if (signed_out_match_iter != signed_out_accounts.end()) { + if (signed_in_accounts.empty()) { + return signed_out_accounts.size() == 1 + ? AccountRelation::NO_SIGNED_IN_SINGLE_SIGNED_OUT_MATCH + : AccountRelation::NO_SIGNED_IN_ONE_OF_SIGNED_OUT_MATCH; + } else { + return AccountRelation::WITH_SIGNED_IN_ONE_OF_SIGNED_OUT_MATCH; + } + } + + return signed_in_accounts.empty() + ? AccountRelation::NO_SIGNED_IN_WITH_SIGNED_OUT_NO_MATCH + : AccountRelation::WITH_SIGNED_IN_NO_MATCH; +} + +void AccountInvestigator::TryPeriodicReport() { + std::vector<ListedAccount> signed_in_accounts, signed_out_accounts; + if (cookie_service_->ListAccounts(&signed_in_accounts, + &signed_out_accounts)) { + DoPeriodicReport(signed_in_accounts, signed_out_accounts); + } else { + periodic_pending_ = true; + } +} + +void AccountInvestigator::DoPeriodicReport( + const std::vector<ListedAccount>& signed_in_accounts, + const std::vector<ListedAccount>& signed_out_accounts) { + SharedCookieJarReport(signed_in_accounts, signed_out_accounts, Time::Now(), + ReportingType::PERIODIC); + + periodic_pending_ = false; + pref_service_->SetDouble(prefs::kGaiaCookiePeriodicReportTime, + Time::Now().ToDoubleT()); + timer_.Start(FROM_HERE, kPeriodicReportingInterval, this, + &AccountInvestigator::TryPeriodicReport); +} + +void AccountInvestigator::SharedCookieJarReport( + const std::vector<ListedAccount>& signed_in_accounts, + const std::vector<ListedAccount>& signed_out_accounts, + const Time now, + const ReportingType type) { + const Time last_changed = Time::FromDoubleT( + pref_service_->GetDouble(prefs::kGaiaCookieChangedTime)); + TimeDelta stable_age; + if (!last_changed.is_null()) + stable_age = std::max(now - last_changed, TimeDelta()); + signin_metrics::LogCookieJarStableAge(stable_age, type); + + int signed_in_count = signed_in_accounts.size(); + int signed_out_count = signed_out_accounts.size(); + signin_metrics::LogCookieJarCounts(signed_in_count, + signed_out_count, + signed_in_count + signed_out_count, type); + + if (signin_manager_->IsAuthenticated()) { + SignedInAccountRelationReport(signed_in_accounts, signed_out_accounts, + type); + } + + // IsShared is defined as true if the local cookie jar contains at least one + // signed out account and a stable age of less than one day. + signin_metrics::LogIsShared( + signed_out_count >= 1 && stable_age < TimeDelta::FromDays(1), type); +} + +void AccountInvestigator::SignedInAccountRelationReport( + const std::vector<ListedAccount>& signed_in_accounts, + const std::vector<ListedAccount>& signed_out_accounts, + ReportingType type) { + signin_metrics::LogAccountRelation( + DiscernRelation(signin_manager_->GetAuthenticatedAccountInfo(), + signed_in_accounts, signed_out_accounts), + type); +} diff --git a/chromium/components/signin/core/browser/account_investigator.h b/chromium/components/signin/core/browser/account_investigator.h new file mode 100644 index 00000000000..477e73c7f17 --- /dev/null +++ b/chromium/components/signin/core/browser/account_investigator.h @@ -0,0 +1,138 @@ +// Copyright 2016 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. + +#ifndef COMPONENTS_SIGNIN_CORE_BROWSER_ACCOUNT_INVESTIGATOR_H_ +#define COMPONENTS_SIGNIN_CORE_BROWSER_ACCOUNT_INVESTIGATOR_H_ + +#include <string> +#include <vector> + +#include "base/macros.h" +#include "base/timer/timer.h" +#include "components/keyed_service/core/keyed_service.h" +#include "components/signin/core/browser/gaia_cookie_manager_service.h" + +struct AccountInfo; +class GaiaCookieManagerService; +class PrefRegistrySimple; +class PrefService; +class SigninManagerBase; + +namespace base { +class Time; +} // namespace base + +namespace signin_metrics { +enum class AccountRelation; +enum class ReportingType; +} // namespace signin_metrics + +// While it is common for the account signed into Chrome and the content area +// to be identical, this is not the only scenario. The purpose of this class +// is to watch for changes in relation between Chrome and content area accounts +// and emit metrics about their relation. +class AccountInvestigator : public KeyedService, + public GaiaCookieManagerService::Observer { + public: + // The targeted interval to perform periodic reporting. If chrome is not + // active at the end of an interval, reporting will be done as soon as + // possible. + static const base::TimeDelta kPeriodicReportingInterval; + + AccountInvestigator(GaiaCookieManagerService* cookie_service, + PrefService* pref_service, + SigninManagerBase* signin_manager); + ~AccountInvestigator() override; + + static void RegisterPrefs(PrefRegistrySimple* registry); + + // Sets up initial state and starts the timer to perform periodic reporting. + void Initialize(); + + // KeyedService: + void Shutdown() override; + + // GaiaCookieManagerService::Observer: + void OnAddAccountToCookieCompleted( + const std::string& account_id, + const GoogleServiceAuthError& error) override; + void OnGaiaAccountsInCookieUpdated( + const std::vector<gaia::ListedAccount>& signed_in_accounts, + const std::vector<gaia::ListedAccount>& signed_out_accounts, + const GoogleServiceAuthError& error) override; + + private: + friend class AccountInvestigatorTest; + + // Calculates the amount of time to wait/delay before the given |interval| has + // passed since |previous|, given |now|. + static base::TimeDelta CalculatePeriodicDelay(base::Time previous, + base::Time now, + base::TimeDelta interval); + + // Calculate a hash of the listed accounts. Order of accounts should not + // affect the hashed values, but signed in and out status should. + static std::string HashAccounts( + const std::vector<gaia::ListedAccount>& signed_in_accounts, + const std::vector<gaia::ListedAccount>& signed_out_accounts); + + // Compares the |info| object, which should correspond to the currently or + // potentially signed into Chrome account, to the various account(s) in the + // given cookie jar. + static signin_metrics::AccountRelation DiscernRelation( + const AccountInfo& info, + const std::vector<gaia::ListedAccount>& signed_in_accounts, + const std::vector<gaia::ListedAccount>& signed_out_accounts); + + // Tries to perform periodic reporting, potentially performing it now if the + // cookie information is cached, otherwise sets things up to perform this + // reporting asynchronously. + void TryPeriodicReport(); + + // Performs periodic reporting with the given cookie jar data and restarts + // the periodic reporting timer. + void DoPeriodicReport( + const std::vector<gaia::ListedAccount>& signed_in_accounts, + const std::vector<gaia::ListedAccount>& signed_out_accounts); + + // Performs the reporting that's shared between the periodic case and the + // on change case. + void SharedCookieJarReport( + const std::vector<gaia::ListedAccount>& signed_in_accounts, + const std::vector<gaia::ListedAccount>& signed_out_accounts, + const base::Time now, + const signin_metrics::ReportingType type); + + // Performs only the account relation reporting, which means comparing the + // signed in account to the cookie jar accounts(s). + void SignedInAccountRelationReport( + const std::vector<gaia::ListedAccount>& signed_in_accounts, + const std::vector<gaia::ListedAccount>& signed_out_accounts, + signin_metrics::ReportingType type); + + GaiaCookieManagerService* cookie_service_; + PrefService* pref_service_; + SigninManagerBase* signin_manager_; + + // Handles invoking our periodic logic at the right time. As part of our + // handling of this call we reset the timer for the next loop. + base::OneShotTimer timer_; + + // If the GaiaCookieManagerService hasn't already cached the cookie data, it + // will not be able to return enough information for us to always perform + // periodic reporting synchronously. This flag is flipped to true when we're + // waiting for this, and will allow us to perform periodic reporting when + // we're called as an observer on a potential cookie jar data change. The + // typical time this is occurs occurs during startup. + bool periodic_pending_ = false; + + // Gets set upon initialization and on potential cookie jar change. This + // allows us ot emit AccountRelation metrics during a sign in that doesn't + // actually change the cookie jar. + bool previously_authenticated_ = false; + + DISALLOW_COPY_AND_ASSIGN(AccountInvestigator); +}; + +#endif // COMPONENTS_SIGNIN_CORE_BROWSER_ACCOUNT_INVESTIGATOR_H_ diff --git a/chromium/components/signin/core/browser/account_investigator_unittest.cc b/chromium/components/signin/core/browser/account_investigator_unittest.cc new file mode 100644 index 00000000000..1b9edd66a47 --- /dev/null +++ b/chromium/components/signin/core/browser/account_investigator_unittest.cc @@ -0,0 +1,371 @@ +// Copyright 2016 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_investigator.h" + +#include <map> +#include <string> +#include <vector> + +#include "base/message_loop/message_loop.h" +#include "base/run_loop.h" +#include "base/test/histogram_tester.h" +#include "base/timer/timer.h" +#include "components/signin/core/browser/account_tracker_service.h" +#include "components/signin/core/browser/fake_gaia_cookie_manager_service.h" +#include "components/signin/core/browser/fake_signin_manager.h" +#include "components/signin/core/browser/signin_metrics.h" +#include "components/signin/core/browser/test_signin_client.h" +#include "components/signin/core/common/signin_pref_names.h" +#include "components/syncable_prefs/testing_pref_service_syncable.h" +#include "google_apis/gaia/gaia_auth_util.h" +#include "google_apis/gaia/gaia_constants.h" +#include "google_apis/gaia/google_service_auth_error.h" +#include "net/url_request/test_url_fetcher_factory.h" +#include "testing/gtest/include/gtest/gtest.h" + +using base::HistogramTester; +using base::Time; +using base::TimeDelta; +using gaia::ListedAccount; +using signin_metrics::AccountRelation; +using signin_metrics::ReportingType; + +class AccountInvestigatorTest : public testing::Test { + protected: + AccountInvestigatorTest() + : signin_client_(&prefs_), + signin_manager_(&signin_client_, + nullptr, + &account_tracker_service_, + nullptr), + gaia_cookie_manager_service_(nullptr, + GaiaConstants::kChromeSource, + &signin_client_), + investigator_(&gaia_cookie_manager_service_, &prefs_, &signin_manager_), + fake_url_fetcher_factory_(nullptr) { + AccountTrackerService::RegisterPrefs(prefs_.registry()); + AccountInvestigator::RegisterPrefs(prefs_.registry()); + SigninManagerBase::RegisterProfilePrefs(prefs_.registry()); + account_tracker_service_.Initialize(&signin_client_); + gaia_cookie_manager_service_.Init(&fake_url_fetcher_factory_); + } + + ~AccountInvestigatorTest() override { investigator_.Shutdown(); } + + FakeSigninManager* signin_manager() { return &signin_manager_; } + PrefService* pref_service() { return &prefs_; } + AccountInvestigator* investigator() { return &investigator_; } + FakeGaiaCookieManagerService* cookie_manager_service() { + return &gaia_cookie_manager_service_; + } + + // Wrappers to invoke private methods through friend class. + TimeDelta Delay(const Time previous, + const Time now, + const TimeDelta interval) { + return AccountInvestigator::CalculatePeriodicDelay(previous, now, interval); + } + std::string Hash(const std::vector<ListedAccount>& signed_in_accounts, + const std::vector<ListedAccount>& signed_out_accounts) { + return AccountInvestigator::HashAccounts(signed_in_accounts, + signed_out_accounts); + } + AccountRelation Relation( + const AccountInfo& info, + const std::vector<ListedAccount>& signed_in_accounts, + const std::vector<ListedAccount>& signed_out_accounts) { + return AccountInvestigator::DiscernRelation(info, + signed_in_accounts, + signed_out_accounts); + } + void SharedReport(const std::vector<gaia::ListedAccount>& signed_in_accounts, + const std::vector<gaia::ListedAccount>& signed_out_accounts, + const Time now, + const ReportingType type) { + investigator_.SharedCookieJarReport(signed_in_accounts, signed_out_accounts, + now, type); + } + void TryPeriodicReport() { investigator_.TryPeriodicReport(); } + bool* periodic_pending() { return &investigator_.periodic_pending_; } + bool* previously_authenticated() { + return &investigator_.previously_authenticated_; + } + base::OneShotTimer* timer() { return &investigator_.timer_; } + + void ExpectRelationReport( + const std::vector<ListedAccount> signed_in_accounts, + const std::vector<ListedAccount> signed_out_accounts, + const ReportingType type, + const AccountRelation expected) { + HistogramTester histogram_tester; + investigator_.SignedInAccountRelationReport(signed_in_accounts, + signed_out_accounts, + type); + ExpectRelationReport(type, histogram_tester, expected); + } + + void ExpectRelationReport(const ReportingType type, + const HistogramTester& histogram_tester, + const AccountRelation expected) { + histogram_tester.ExpectUniqueSample( + "Signin.CookieJar.ChromeAccountRelation" + suffix_[type], + static_cast<int>(expected), 1); + } + + // If |relation| is a nullptr, then it should not have been recorded. + // If |stable_age| is a nullptr, then we're not sure what the expected time + // should have been, but it still should have been recorded. + void ExpectSharedReportHistograms(const ReportingType type, + const HistogramTester& histogram_tester, + const TimeDelta* stable_age, + const int signed_in_count, + const int signed_out_count, + const int total_count, + const AccountRelation* relation, + const bool is_shared) { + if (stable_age) { + histogram_tester.ExpectUniqueSample( + "Signin.CookieJar.StableAge" + suffix_[type], stable_age->InSeconds(), + 1); + } else { + histogram_tester.ExpectTotalCount( + "Signin.CookieJar.StableAge" + suffix_[type], 1); + } + histogram_tester.ExpectUniqueSample( + "Signin.CookieJar.SignedInCount" + suffix_[type], signed_in_count, 1); + histogram_tester.ExpectUniqueSample( + "Signin.CookieJar.SignedOutCount" + suffix_[type], signed_out_count, 1); + histogram_tester.ExpectUniqueSample( + "Signin.CookieJar.TotalCount" + suffix_[type], total_count, 1); + if (relation) { + histogram_tester.ExpectUniqueSample( + "Signin.CookieJar.ChromeAccountRelation" + suffix_[type], + static_cast<int>(*relation), 1); + } else { + histogram_tester.ExpectTotalCount( + "Signin.CookieJar.ChromeAccountRelation" + suffix_[type], 0); + } + histogram_tester.ExpectUniqueSample("Signin.IsShared" + suffix_[type], + is_shared, 1); + } + + private: + // Timer needs a message loop. + base::MessageLoop message_loop_; + syncable_prefs::TestingPrefServiceSyncable prefs_; + AccountTrackerService account_tracker_service_; + TestSigninClient signin_client_; + FakeSigninManager signin_manager_; + FakeGaiaCookieManagerService gaia_cookie_manager_service_; + AccountInvestigator investigator_; + net::FakeURLFetcherFactory fake_url_fetcher_factory_; + std::map<ReportingType, std::string> suffix_ = { + {ReportingType::PERIODIC, "_Periodic"}, + {ReportingType::ON_CHANGE, "_OnChange"}}; +}; + +namespace { + +ListedAccount Account(const std::string& id) { + ListedAccount account; + account.id = id; + return account; +} + +AccountInfo Info(const std::string& id) { + AccountInfo info; + info.account_id = id; + return info; +} + +const std::vector<ListedAccount> no_accounts{}; +const std::vector<ListedAccount> just_one{Account("1")}; +const std::vector<ListedAccount> just_two{Account("2")}; +const std::vector<ListedAccount> both{Account("1"), Account("2")}; +const std::vector<ListedAccount> both_reversed{Account("2"), Account("1")}; + +const AccountInfo one(Info("1")); +const AccountInfo three(Info("3")); + +TEST_F(AccountInvestigatorTest, CalculatePeriodicDelay) { + const Time epoch; + const TimeDelta day(TimeDelta::FromDays(1)); + const TimeDelta big(TimeDelta::FromDays(1000)); + + EXPECT_EQ(day, Delay(epoch, epoch, day)); + EXPECT_EQ(day, Delay(epoch + big, epoch + big, day)); + EXPECT_EQ(TimeDelta(), Delay(epoch, epoch + big, day)); + EXPECT_EQ(day, Delay(epoch + big, epoch, day)); + EXPECT_EQ(day, Delay(epoch, epoch + day, TimeDelta::FromDays(2))); +} + +TEST_F(AccountInvestigatorTest, HashAccounts) { + EXPECT_EQ(Hash(no_accounts, no_accounts), Hash(no_accounts, no_accounts)); + EXPECT_EQ(Hash(just_one, just_two), Hash(just_one, just_two)); + EXPECT_EQ(Hash(both, no_accounts), Hash(both, no_accounts)); + EXPECT_EQ(Hash(no_accounts, both), Hash(no_accounts, both)); + EXPECT_EQ(Hash(both, no_accounts), Hash(both_reversed, no_accounts)); + EXPECT_EQ(Hash(no_accounts, both), Hash(no_accounts, both_reversed)); + + EXPECT_NE(Hash(no_accounts, no_accounts), Hash(just_one, no_accounts)); + EXPECT_NE(Hash(no_accounts, no_accounts), Hash(no_accounts, just_one)); + EXPECT_NE(Hash(just_one, no_accounts), Hash(just_two, no_accounts)); + EXPECT_NE(Hash(just_one, no_accounts), Hash(both, no_accounts)); + EXPECT_NE(Hash(just_one, no_accounts), Hash(no_accounts, just_one)); +} + +TEST_F(AccountInvestigatorTest, DiscernRelation) { + EXPECT_EQ(AccountRelation::EMPTY_COOKIE_JAR, + Relation(one, no_accounts, no_accounts)); + EXPECT_EQ(AccountRelation::SINGLE_SIGNED_IN_MATCH_NO_SIGNED_OUT, + Relation(one, just_one, no_accounts)); + EXPECT_EQ(AccountRelation::SINGLE_SINGED_IN_MATCH_WITH_SIGNED_OUT, + Relation(one, just_one, just_two)); + EXPECT_EQ(AccountRelation::WITH_SIGNED_IN_NO_MATCH, + Relation(one, just_two, no_accounts)); + EXPECT_EQ(AccountRelation::ONE_OF_SIGNED_IN_MATCH_ANY_SIGNED_OUT, + Relation(one, both, just_one)); + EXPECT_EQ(AccountRelation::ONE_OF_SIGNED_IN_MATCH_ANY_SIGNED_OUT, + Relation(one, both, no_accounts)); + EXPECT_EQ(AccountRelation::NO_SIGNED_IN_ONE_OF_SIGNED_OUT_MATCH, + Relation(one, no_accounts, both)); + EXPECT_EQ(AccountRelation::NO_SIGNED_IN_SINGLE_SIGNED_OUT_MATCH, + Relation(one, no_accounts, just_one)); + EXPECT_EQ(AccountRelation::WITH_SIGNED_IN_ONE_OF_SIGNED_OUT_MATCH, + Relation(one, just_two, just_one)); + EXPECT_EQ(AccountRelation::NO_SIGNED_IN_WITH_SIGNED_OUT_NO_MATCH, + Relation(three, no_accounts, both)); +} + +TEST_F(AccountInvestigatorTest, SignedInAccountRelationReport) { + ExpectRelationReport(just_one, no_accounts, ReportingType::PERIODIC, + AccountRelation::WITH_SIGNED_IN_NO_MATCH); + signin_manager()->SignIn("1", "a", "A"); + ExpectRelationReport(just_one, no_accounts, ReportingType::PERIODIC, + AccountRelation::SINGLE_SIGNED_IN_MATCH_NO_SIGNED_OUT); + ExpectRelationReport(just_two, no_accounts, ReportingType::ON_CHANGE, + AccountRelation::WITH_SIGNED_IN_NO_MATCH); +} + +TEST_F(AccountInvestigatorTest, SharedCookieJarReportEmpty) { + const HistogramTester histogram_tester; + const TimeDelta expected_stable_age; + SharedReport(no_accounts, no_accounts, Time(), ReportingType::PERIODIC); + ExpectSharedReportHistograms(ReportingType::PERIODIC, histogram_tester, + &expected_stable_age, 0, 0, 0, nullptr, false); +} + +TEST_F(AccountInvestigatorTest, SharedCookieJarReportWithAccount) { + signin_manager()->SignIn("1", "a", "A"); + base::Time now = base::Time::Now(); + pref_service()->SetDouble(prefs::kGaiaCookieChangedTime, now.ToDoubleT()); + const AccountRelation expected_relation( + AccountRelation::ONE_OF_SIGNED_IN_MATCH_ANY_SIGNED_OUT); + const HistogramTester histogram_tester; + const TimeDelta expected_stable_age(TimeDelta::FromDays(1)); + SharedReport(both, no_accounts, now + TimeDelta::FromDays(1), + ReportingType::ON_CHANGE); + ExpectSharedReportHistograms(ReportingType::ON_CHANGE, histogram_tester, + &expected_stable_age, 2, 0, 2, + &expected_relation, false); +} + +TEST_F(AccountInvestigatorTest, OnGaiaAccountsInCookieUpdatedError) { + const HistogramTester histogram_tester; + GoogleServiceAuthError error(GoogleServiceAuthError::SERVICE_UNAVAILABLE); + investigator()->OnGaiaAccountsInCookieUpdated(just_one, no_accounts, error); + EXPECT_EQ(0u, histogram_tester.GetTotalCountsForPrefix("Signin.").size()); +} + +TEST_F(AccountInvestigatorTest, OnGaiaAccountsInCookieUpdatedOnChange) { + const HistogramTester histogram_tester; + investigator()->OnGaiaAccountsInCookieUpdated( + just_one, no_accounts, GoogleServiceAuthError::AuthErrorNone()); + ExpectSharedReportHistograms(ReportingType::ON_CHANGE, histogram_tester, + nullptr, 1, 0, 1, nullptr, false); +} + +TEST_F(AccountInvestigatorTest, OnGaiaAccountsInCookieUpdatedSigninOnly) { + // Initial update to simulate the update on first-time-run. + investigator()->OnGaiaAccountsInCookieUpdated( + no_accounts, no_accounts, GoogleServiceAuthError::AuthErrorNone()); + + const HistogramTester histogram_tester; + signin_manager()->SignIn("1", "a", "A"); + pref_service()->SetString(prefs::kGaiaCookieHash, + Hash(just_one, no_accounts)); + investigator()->OnGaiaAccountsInCookieUpdated( + just_one, no_accounts, GoogleServiceAuthError::AuthErrorNone()); + EXPECT_EQ(1u, histogram_tester.GetTotalCountsForPrefix("Signin.").size()); + ExpectRelationReport(ReportingType::ON_CHANGE, histogram_tester, + AccountRelation::SINGLE_SIGNED_IN_MATCH_NO_SIGNED_OUT); +} + +TEST_F(AccountInvestigatorTest, + OnGaiaAccountsInCookieUpdatedSigninSignOutOfContent) { + const HistogramTester histogram_tester; + signin_manager()->SignIn("1", "a", "A"); + investigator()->OnGaiaAccountsInCookieUpdated( + just_one, no_accounts, GoogleServiceAuthError::AuthErrorNone()); + ExpectRelationReport(ReportingType::ON_CHANGE, histogram_tester, + AccountRelation::SINGLE_SIGNED_IN_MATCH_NO_SIGNED_OUT); + + // Simulate a sign out of the content area. + const HistogramTester histogram_tester2; + investigator()->OnGaiaAccountsInCookieUpdated( + no_accounts, just_one, GoogleServiceAuthError::AuthErrorNone()); + const AccountRelation expected_relation = + AccountRelation::NO_SIGNED_IN_SINGLE_SIGNED_OUT_MATCH; + ExpectSharedReportHistograms(ReportingType::ON_CHANGE, histogram_tester2, + nullptr, 0, 1, 1, &expected_relation, true); +} + +TEST_F(AccountInvestigatorTest, Initialize) { + EXPECT_FALSE(*previously_authenticated()); + EXPECT_FALSE(timer()->IsRunning()); + + investigator()->Initialize(); + EXPECT_FALSE(*previously_authenticated()); + EXPECT_TRUE(timer()->IsRunning()); + + investigator()->Shutdown(); + EXPECT_FALSE(timer()->IsRunning()); +} + +TEST_F(AccountInvestigatorTest, InitializeSignedIn) { + signin_manager()->SignIn("1", "a", "A"); + EXPECT_FALSE(*previously_authenticated()); + + investigator()->Initialize(); + EXPECT_TRUE(*previously_authenticated()); +} + +TEST_F(AccountInvestigatorTest, TryPeriodicReportStale) { + investigator()->Initialize(); + cookie_manager_service()->set_list_accounts_stale_for_testing(true); + cookie_manager_service()->SetListAccountsResponseOneAccount("f@bar.com", "1"); + + const HistogramTester histogram_tester; + TryPeriodicReport(); + EXPECT_TRUE(*periodic_pending()); + EXPECT_EQ(0u, histogram_tester.GetTotalCountsForPrefix("Signin.").size()); + + base::RunLoop().RunUntilIdle(); + EXPECT_FALSE(*periodic_pending()); + ExpectSharedReportHistograms(ReportingType::PERIODIC, histogram_tester, + nullptr, 1, 0, 1, nullptr, false); +} + +TEST_F(AccountInvestigatorTest, TryPeriodicReportEmpty) { + cookie_manager_service()->set_list_accounts_stale_for_testing(false); + const HistogramTester histogram_tester; + + TryPeriodicReport(); + EXPECT_FALSE(*periodic_pending()); + ExpectSharedReportHistograms(ReportingType::PERIODIC, histogram_tester, + nullptr, 0, 0, 0, nullptr, false); +} + +} // namespace diff --git a/chromium/components/signin/core/browser/account_reconcilor.cc b/chromium/components/signin/core/browser/account_reconcilor.cc index 7c9c7f2333e..1a924082d25 100644 --- a/chromium/components/signin/core/browser/account_reconcilor.cc +++ b/chromium/components/signin/core/browser/account_reconcilor.cc @@ -280,15 +280,23 @@ void AccountReconcilor::StartReconcile() { is_reconcile_started_ = true; error_during_last_reconcile_ = false; + // ListAccounts() also gets signed out accounts but this class doesn't use + // them. + std::vector<gaia::ListedAccount> signed_out_accounts; + // Rely on the GCMS to manage calls to and responses from ListAccounts. - if (cookie_manager_service_->ListAccounts(&gaia_accounts_)) { + if (cookie_manager_service_->ListAccounts(&gaia_accounts_, + &signed_out_accounts)) { OnGaiaAccountsInCookieUpdated( - gaia_accounts_, GoogleServiceAuthError(GoogleServiceAuthError::NONE)); + gaia_accounts_, + signed_out_accounts, + GoogleServiceAuthError(GoogleServiceAuthError::NONE)); } } void AccountReconcilor::OnGaiaAccountsInCookieUpdated( const std::vector<gaia::ListedAccount>& accounts, + const std::vector<gaia::ListedAccount>& signed_out_accounts, const GoogleServiceAuthError& error) { VLOG(1) << "AccountReconcilor::OnGaiaAccountsInCookieUpdated: " << "CookieJar " << accounts.size() << " accounts, " diff --git a/chromium/components/signin/core/browser/account_reconcilor.h b/chromium/components/signin/core/browser/account_reconcilor.h index 7a9fc392fb3..68ad603880c 100644 --- a/chromium/components/signin/core/browser/account_reconcilor.h +++ b/chromium/components/signin/core/browser/account_reconcilor.h @@ -147,6 +147,7 @@ class AccountReconcilor : public KeyedService, const GoogleServiceAuthError& error) override; void OnGaiaAccountsInCookieUpdated( const std::vector<gaia::ListedAccount>& accounts, + const std::vector<gaia::ListedAccount>& signed_out_accounts, const GoogleServiceAuthError& error) override; // Overriden from OAuth2TokenService::Observer. diff --git a/chromium/components/signin/core/browser/account_tracker_service.cc b/chromium/components/signin/core/browser/account_tracker_service.cc index 12965902a88..b1dc370db2b 100644 --- a/chromium/components/signin/core/browser/account_tracker_service.cc +++ b/chromium/components/signin/core/browser/account_tracker_service.cc @@ -348,7 +348,7 @@ void AccountTrackerService::LoadFromPrefs() { if (dict->GetList(kAccountServiceFlagsPath, &service_flags_list)) { contains_deprecated_service_flags = true; std::string flag_string; - for (base::Value* flag : *service_flags_list) { + for (const auto& flag : *service_flags_list) { if (flag->GetAsString(&flag_string) && flag_string == kChildAccountServiceFlag) { is_child_account = true; diff --git a/chromium/components/signin/core/browser/fake_gaia_cookie_manager_service.cc b/chromium/components/signin/core/browser/fake_gaia_cookie_manager_service.cc new file mode 100644 index 00000000000..8a0249cd615 --- /dev/null +++ b/chromium/components/signin/core/browser/fake_gaia_cookie_manager_service.cc @@ -0,0 +1,109 @@ +// 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 "components/signin/core/browser/fake_gaia_cookie_manager_service.h" + +#include "base/memory/ptr_util.h" +#include "base/strings/stringprintf.h" +#include "components/signin/core/browser/profile_oauth2_token_service.h" +#include "google_apis/gaia/gaia_constants.h" +#include "google_apis/gaia/gaia_urls.h" + +FakeGaiaCookieManagerService::FakeGaiaCookieManagerService( + OAuth2TokenService* token_service, + const std::string& source, + SigninClient* client) + : GaiaCookieManagerService(token_service, source, client), + url_fetcher_factory_(NULL) {} + +void FakeGaiaCookieManagerService::Init( + net::FakeURLFetcherFactory* url_fetcher_factory) { + url_fetcher_factory_ = url_fetcher_factory; +} + +void FakeGaiaCookieManagerService::SetListAccountsResponseHttpNotFound() { + DCHECK(url_fetcher_factory_); + url_fetcher_factory_->SetFakeResponse( + GaiaUrls::GetInstance()->ListAccountsURLWithSource( + GaiaConstants::kChromeSource), + "", net::HTTP_NOT_FOUND, net::URLRequestStatus::SUCCESS); +} + +void FakeGaiaCookieManagerService::SetListAccountsResponseWebLoginRequired() { + DCHECK(url_fetcher_factory_); + url_fetcher_factory_->SetFakeResponse( + GaiaUrls::GetInstance()->ListAccountsURLWithSource( + GaiaConstants::kChromeSource), + "Info=WebLoginRequired", net::HTTP_OK, net::URLRequestStatus::SUCCESS); +} + +void FakeGaiaCookieManagerService::SetListAccountsResponseNoAccounts() { + DCHECK(url_fetcher_factory_); + url_fetcher_factory_->SetFakeResponse( + GaiaUrls::GetInstance()->ListAccountsURLWithSource( + GaiaConstants::kChromeSource), + "[\"f\", []]", net::HTTP_OK, net::URLRequestStatus::SUCCESS); +} + +void FakeGaiaCookieManagerService::SetListAccountsResponseOneAccount( + const char* email, + const char* gaia_id) { + DCHECK(url_fetcher_factory_); + url_fetcher_factory_->SetFakeResponse( + GaiaUrls::GetInstance()->ListAccountsURLWithSource( + GaiaConstants::kChromeSource), + base::StringPrintf( + "[\"f\", [[\"b\", 0, \"n\", \"%s\", \"p\", 0, 0, 0, 0, 1, \"%s\"]]]", + email, gaia_id), + net::HTTP_OK, net::URLRequestStatus::SUCCESS); +} + +void FakeGaiaCookieManagerService::SetListAccountsResponseOneAccountWithExpiry( + const char* email, + const char* gaia_id, + bool expired) { + DCHECK(url_fetcher_factory_); + url_fetcher_factory_->SetFakeResponse( + GaiaUrls::GetInstance()->ListAccountsURLWithSource( + GaiaConstants::kChromeSource), + base::StringPrintf( + "[\"f\", [[\"b\", 0, \"n\", \"%s\", \"p\", 0, 0, 0, 0, %d, \"%s\"]]]", + email, expired ? 0 : 1, gaia_id), + net::HTTP_OK, net::URLRequestStatus::SUCCESS); +} + +void FakeGaiaCookieManagerService::SetListAccountsResponseTwoAccounts( + const char* email1, + const char* gaia_id1, + const char* email2, + const char* gaia_id2) { + DCHECK(url_fetcher_factory_); + url_fetcher_factory_->SetFakeResponse( + GaiaUrls::GetInstance()->ListAccountsURLWithSource( + GaiaConstants::kChromeSource), + base::StringPrintf( + "[\"f\", [[\"b\", 0, \"n\", \"%s\", \"p\", 0, 0, 0, 0, 1, \"%s\"], " + "[\"b\", 0, \"n\", \"%s\", \"p\", 0, 0, 0, 0, 1, \"%s\"]]]", + email1, gaia_id1, email2, gaia_id2), + net::HTTP_OK, net::URLRequestStatus::SUCCESS); +} + +void FakeGaiaCookieManagerService::SetListAccountsResponseTwoAccountsWithExpiry( + const char* email1, + const char* gaia_id1, + bool account1_expired, + const char* email2, + const char* gaia_id2, + bool account2_expired) { + DCHECK(url_fetcher_factory_); + url_fetcher_factory_->SetFakeResponse( + GaiaUrls::GetInstance()->ListAccountsURLWithSource( + GaiaConstants::kChromeSource), + base::StringPrintf( + "[\"f\", [[\"b\", 0, \"n\", \"%s\", \"p\", 0, 0, 0, 0, %d, \"%s\"], " + "[\"b\", 0, \"n\", \"%s\", \"p\", 0, 0, 0, 0, %d, \"%s\"]]]", + email1, account1_expired ? 0 : 1, gaia_id1, email2, + account2_expired ? 0 : 1, gaia_id2), + net::HTTP_OK, net::URLRequestStatus::SUCCESS); +} diff --git a/chromium/components/signin/core/browser/fake_gaia_cookie_manager_service.h b/chromium/components/signin/core/browser/fake_gaia_cookie_manager_service.h new file mode 100644 index 00000000000..cc79071e440 --- /dev/null +++ b/chromium/components/signin/core/browser/fake_gaia_cookie_manager_service.h @@ -0,0 +1,52 @@ +// 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. +#ifndef COMPONENTS_SIGNIN_CORE_BROWSER_FAKE_GAIA_COOKIE_MANAGER_SERVICE_H_ +#define COMPONENTS_SIGNIN_CORE_BROWSER_FAKE_GAIA_COOKIE_MANAGER_SERVICE_H_ + +#include <memory> +#include <string> + +#include "base/macros.h" +#include "components/signin/core/browser/gaia_cookie_manager_service.h" +#include "net/url_request/test_url_fetcher_factory.h" + +namespace content { +class BrowserContext; +} + +class FakeGaiaCookieManagerService : public GaiaCookieManagerService { + public: + FakeGaiaCookieManagerService(OAuth2TokenService* token_service, + const std::string& source, + SigninClient* client); + + void Init(net::FakeURLFetcherFactory* url_fetcher_factory); + + void SetListAccountsResponseHttpNotFound(); + void SetListAccountsResponseWebLoginRequired(); + void SetListAccountsResponseNoAccounts(); + void SetListAccountsResponseOneAccount(const char* email, + const char* gaia_id); + void SetListAccountsResponseOneAccountWithExpiry(const char* account, + const char* gaia_id, + bool expired); + void SetListAccountsResponseTwoAccounts(const char* email1, + const char* gaia_id1, + const char* email2, + const char* gaia_id2); + void SetListAccountsResponseTwoAccountsWithExpiry(const char* email1, + const char* gaia_id1, + bool account1_expired, + const char* email2, + const char* gaia_id2, + bool account2_expired); + + private: + // Provide a fake response for calls to /ListAccounts. + net::FakeURLFetcherFactory* url_fetcher_factory_; + + DISALLOW_COPY_AND_ASSIGN(FakeGaiaCookieManagerService); +}; + +#endif // COMPONENTS_SIGNIN_CORE_BROWSER_FAKE_GAIA_COOKIE_MANAGER_SERVICE_H_ diff --git a/chromium/components/signin/core/browser/gaia_cookie_manager_service.cc b/chromium/components/signin/core/browser/gaia_cookie_manager_service.cc index 19f3f6d42d2..d6b4f01106f 100644 --- a/chromium/components/signin/core/browser/gaia_cookie_manager_service.cc +++ b/chromium/components/signin/core/browser/gaia_cookie_manager_service.cc @@ -343,12 +343,17 @@ void GaiaCookieManagerService::AddAccountToCookieWithToken( } bool GaiaCookieManagerService::ListAccounts( - std::vector<gaia::ListedAccount>* accounts) { - DCHECK(accounts); - accounts->clear(); - + std::vector<gaia::ListedAccount>* accounts, + std::vector<gaia::ListedAccount>* signed_out_accounts) { if (!list_accounts_stale_) { - accounts->assign(listed_accounts_.begin(), listed_accounts_.end()); + if (accounts) + accounts->assign(listed_accounts_.begin(), listed_accounts_.end()); + + if (signed_out_accounts) { + signed_out_accounts->assign(signed_out_accounts_.begin(), + signed_out_accounts_.end()); + } + return true; } @@ -555,8 +560,10 @@ void GaiaCookieManagerService::OnListAccountsSuccess(const std::string& data) { GaiaCookieRequestType::LIST_ACCOUNTS); fetcher_backoff_.InformOfRequest(true); - if (!gaia::ParseListAccountsData(data, &listed_accounts_)) { + if (!gaia::ParseListAccountsData( + data, &listed_accounts_, &signed_out_accounts_)) { listed_accounts_.clear(); + signed_out_accounts_.clear(); OnListAccountsFailure(GoogleServiceAuthError( GoogleServiceAuthError::UNEXPECTED_SERVICE_RESPONSE)); return; @@ -577,6 +584,7 @@ void GaiaCookieManagerService::OnListAccountsSuccess(const std::string& data) { FOR_EACH_OBSERVER(Observer, observer_list_, OnGaiaAccountsInCookieUpdated( listed_accounts_, + signed_out_accounts_, GoogleServiceAuthError(GoogleServiceAuthError::NONE))); } @@ -602,7 +610,8 @@ void GaiaCookieManagerService::OnListAccountsFailure( UMA_HISTOGRAM_ENUMERATION("Signin.ListAccountsFailure", error.state(), GoogleServiceAuthError::NUM_STATES); FOR_EACH_OBSERVER(Observer, observer_list_, - OnGaiaAccountsInCookieUpdated(listed_accounts_, error)); + OnGaiaAccountsInCookieUpdated( + listed_accounts_, signed_out_accounts_, error)); HandleNextRequest(); } diff --git a/chromium/components/signin/core/browser/gaia_cookie_manager_service.h b/chromium/components/signin/core/browser/gaia_cookie_manager_service.h index 60ba2e0f018..f14f1cae0f9 100644 --- a/chromium/components/signin/core/browser/gaia_cookie_manager_service.h +++ b/chromium/components/signin/core/browser/gaia_cookie_manager_service.h @@ -84,6 +84,7 @@ class GaiaCookieManagerService : public KeyedService, // is passed in |error|. virtual void OnGaiaAccountsInCookieUpdated( const std::vector<gaia::ListedAccount>& accounts, + const std::vector<gaia::ListedAccount>& signed_out_accounts, const GoogleServiceAuthError& error) {} protected: @@ -168,7 +169,10 @@ class GaiaCookieManagerService : public KeyedService, // parameter if return is false). The parameter will be assigned the current // cached accounts. If the accounts are not up to date, a ListAccounts fetch // is sent GAIA and Observer::OnGaiaAccountsInCookieUpdated will be called. - bool ListAccounts(std::vector<gaia::ListedAccount>* accounts); + // If either of |accounts| or |signed_out_accounts| is null, the corresponding + // accounts returned from /ListAccounts are ignored. + bool ListAccounts(std::vector<gaia::ListedAccount>* accounts, + std::vector<gaia::ListedAccount>* signed_out_accounts); // Triggers a ListAccounts fetch. This is public so that callers that know // that a check which GAIA should be done can force it. @@ -290,6 +294,7 @@ class GaiaCookieManagerService : public KeyedService, bool external_cc_result_fetched_; std::vector<gaia::ListedAccount> listed_accounts_; + std::vector<gaia::ListedAccount> signed_out_accounts_; bool list_accounts_stale_; diff --git a/chromium/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc b/chromium/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc index 9787144b1ca..9e5f896f956 100644 --- a/chromium/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc +++ b/chromium/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc @@ -13,6 +13,7 @@ #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/message_loop/message_loop.h" +#include "base/run_loop.h" #include "base/single_thread_task_runner.h" #include "base/strings/stringprintf.h" #include "base/test/histogram_tester.h" @@ -42,8 +43,9 @@ class MockObserver : public GaiaCookieManagerService::Observer { MOCK_METHOD2(OnAddAccountToCookieCompleted, void(const std::string&, const GoogleServiceAuthError&)); - MOCK_METHOD2(OnGaiaAccountsInCookieUpdated, + MOCK_METHOD3(OnGaiaAccountsInCookieUpdated, void(const std::vector<gaia::ListedAccount>&, + const std::vector<gaia::ListedAccount>&, const GoogleServiceAuthError&)); private: GaiaCookieManagerService* helper_; @@ -225,7 +227,7 @@ TEST_F(GaiaCookieManagerServiceTest, MergeSessionRetried) { base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( FROM_HERE, base::MessageLoop::QuitWhenIdleClosure(), base::TimeDelta::FromMilliseconds(1100)); - base::MessageLoop::current()->Run(); + base::RunLoop().Run(); SimulateMergeSessionSuccess(&helper, "token"); DCHECK(!helper.is_running()); } @@ -249,7 +251,7 @@ TEST_F(GaiaCookieManagerServiceTest, MergeSessionRetriedTwice) { base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( FROM_HERE, base::MessageLoop::QuitWhenIdleClosure(), base::TimeDelta::FromMilliseconds(1100)); - base::MessageLoop::current()->Run(); + base::RunLoop().Run(); SimulateMergeSessionFailure(&helper, canceled()); DCHECK(helper.is_running()); // Next transient error incurs a retry after 3 seconds. @@ -258,7 +260,7 @@ TEST_F(GaiaCookieManagerServiceTest, MergeSessionRetriedTwice) { base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( FROM_HERE, base::MessageLoop::QuitWhenIdleClosure(), base::TimeDelta::FromMilliseconds(3100)); - base::MessageLoop::current()->Run(); + base::RunLoop().Run(); SimulateMergeSessionSuccess(&helper, "token"); DCHECK(!helper.is_running()); histograms.ExpectUniqueSample("OAuth2Login.MergeSessionRetry", @@ -532,11 +534,13 @@ TEST_F(GaiaCookieManagerServiceTest, ListAccountsFirstReturnsEmpty) { MockObserver observer(&helper); std::vector<gaia::ListedAccount> list_accounts; + std::vector<gaia::ListedAccount> signed_out_accounts; EXPECT_CALL(helper, StartFetchingListAccounts()); - ASSERT_FALSE(helper.ListAccounts(&list_accounts)); + ASSERT_FALSE(helper.ListAccounts(&list_accounts, &signed_out_accounts)); ASSERT_TRUE(list_accounts.empty()); + ASSERT_TRUE(signed_out_accounts.empty()); } TEST_F(GaiaCookieManagerServiceTest, ListAccountsFindsOneAccount) { @@ -550,44 +554,113 @@ TEST_F(GaiaCookieManagerServiceTest, ListAccountsFindsOneAccount) { listed_account.raw_email = "a@b.com"; listed_account.gaia_id = "8"; listed_account.valid = true; + listed_account.signed_out = false; expected_accounts.push_back(listed_account); + std::vector<gaia::ListedAccount> signed_out_accounts; + std::vector<gaia::ListedAccount> expected_signed_out_accounts; + EXPECT_CALL(helper, StartFetchingListAccounts()); - EXPECT_CALL(observer, OnGaiaAccountsInCookieUpdated(expected_accounts, - no_error())); + EXPECT_CALL(observer, OnGaiaAccountsInCookieUpdated( + expected_accounts, expected_signed_out_accounts, no_error())); - ASSERT_FALSE(helper.ListAccounts(&list_accounts)); + ASSERT_FALSE(helper.ListAccounts(&list_accounts, &signed_out_accounts)); SimulateListAccountsSuccess(&helper, "[\"f\", [[\"b\", 0, \"n\", \"a@b.com\", \"p\", 0, 0, 0, 0, 1, \"8\"]]]"); } +TEST_F(GaiaCookieManagerServiceTest, ListAccountsFindsSignedOutAccounts) { + InstrumentedGaiaCookieManagerService helper(token_service(), signin_client()); + MockObserver observer(&helper); + + std::vector<gaia::ListedAccount> list_accounts; + std::vector<gaia::ListedAccount> expected_accounts; + gaia::ListedAccount listed_account; + listed_account.email = "a@b.com"; + listed_account.raw_email = "a@b.com"; + listed_account.gaia_id = "8"; + listed_account.valid = true; + listed_account.signed_out = false; + expected_accounts.push_back(listed_account); + + std::vector<gaia::ListedAccount> signed_out_accounts; + std::vector<gaia::ListedAccount> expected_signed_out_accounts; + gaia::ListedAccount signed_out_account; + signed_out_account.email = "c@d.com"; + signed_out_account.raw_email = "c@d.com"; + signed_out_account.gaia_id = "9"; + signed_out_account.valid = true; + signed_out_account.signed_out = true; + expected_signed_out_accounts.push_back(signed_out_account); + + EXPECT_CALL(helper, StartFetchingListAccounts()); + EXPECT_CALL(observer, OnGaiaAccountsInCookieUpdated( + expected_accounts, expected_signed_out_accounts, no_error())); + + ASSERT_FALSE(helper.ListAccounts(&list_accounts, &signed_out_accounts)); + + SimulateListAccountsSuccess(&helper, + "[\"f\"," + "[[\"b\", 0, \"n\", \"a@b.com\", \"p\", 0, 0, 0, 0, 1, \"8\"]," + " [\"b\", 0, \"n\", \"c@d.com\", \"p\", 0, 0, 0, 0, 1, \"9\"," + "null,null,null,1]]]"); +} + +TEST_F(GaiaCookieManagerServiceTest, ListAccountsAcceptsNull) { + InstrumentedGaiaCookieManagerService helper(token_service(), signin_client()); + MockObserver observer(&helper); + + ASSERT_FALSE(helper.ListAccounts(nullptr, nullptr)); + + SimulateListAccountsSuccess(&helper, + "[\"f\"," + "[[\"b\", 0, \"n\", \"a@b.com\", \"p\", 0, 0, 0, 0, 1, \"8\"]," + " [\"b\", 0, \"n\", \"c@d.com\", \"p\", 0, 0, 0, 0, 1, \"9\"," + "null,null,null,1]]]"); + + std::vector<gaia::ListedAccount> signed_out_accounts; + ASSERT_TRUE(helper.ListAccounts(nullptr, &signed_out_accounts)); + ASSERT_EQ(1u, signed_out_accounts.size()); + + std::vector<gaia::ListedAccount> accounts; + ASSERT_TRUE(helper.ListAccounts(&accounts, nullptr)); + ASSERT_EQ(1u, accounts.size()); +} + TEST_F(GaiaCookieManagerServiceTest, ListAccountsAfterOnCookieChanged) { InstrumentedGaiaCookieManagerService helper(token_service(), signin_client()); MockObserver observer(&helper); std::vector<gaia::ListedAccount> list_accounts; std::vector<gaia::ListedAccount> empty_list_accounts; + std::vector<gaia::ListedAccount> signed_out_accounts; + std::vector<gaia::ListedAccount> empty_signed_out_accounts; EXPECT_CALL(helper, StartFetchingListAccounts()); EXPECT_CALL(observer, - OnGaiaAccountsInCookieUpdated(empty_list_accounts, no_error())); - ASSERT_FALSE(helper.ListAccounts(&list_accounts)); + OnGaiaAccountsInCookieUpdated( + empty_list_accounts, empty_signed_out_accounts, no_error())); + ASSERT_FALSE(helper.ListAccounts(&list_accounts, &signed_out_accounts)); ASSERT_TRUE(list_accounts.empty()); + ASSERT_TRUE(signed_out_accounts.empty()); SimulateListAccountsSuccess(&helper, "[\"f\",[]]"); // ListAccounts returns cached data. - ASSERT_TRUE(helper.ListAccounts(&list_accounts)); + ASSERT_TRUE(helper.ListAccounts(&list_accounts, &signed_out_accounts)); ASSERT_TRUE(list_accounts.empty()); + ASSERT_TRUE(signed_out_accounts.empty()); EXPECT_CALL(helper, StartFetchingListAccounts()); EXPECT_CALL(observer, - OnGaiaAccountsInCookieUpdated(empty_list_accounts, no_error())); + OnGaiaAccountsInCookieUpdated( + empty_list_accounts, empty_signed_out_accounts, no_error())); helper.ForceOnCookieChangedProcessing(); // OnCookieChanged should invalidate cached data. - ASSERT_FALSE(helper.ListAccounts(&list_accounts)); + ASSERT_FALSE(helper.ListAccounts(&list_accounts, &signed_out_accounts)); ASSERT_TRUE(list_accounts.empty()); + ASSERT_TRUE(signed_out_accounts.empty()); SimulateListAccountsSuccess(&helper, "[\"f\",[]]"); } diff --git a/chromium/components/signin/core/browser/signin_metrics.cc b/chromium/components/signin/core/browser/signin_metrics.cc index aa5231219ac..279371a6024 100644 --- a/chromium/components/signin/core/browser/signin_metrics.cc +++ b/chromium/components/signin/core/browser/signin_metrics.cc @@ -166,4 +166,35 @@ void LogAccountEquality(AccountEquality equality) { static_cast<int>(AccountEquality::HISTOGRAM_COUNT)); } +void LogCookieJarStableAge(const base::TimeDelta stable_age, + const ReportingType type) { + INVESTIGATOR_HISTOGRAM_CUSTOM_COUNTS( + "Signin.CookieJar.StableAge", type, stable_age.InSeconds(), 1, + base::TimeDelta::FromDays(365).InSeconds(), 100); +} + +void LogCookieJarCounts(const int signed_in, + const int signed_out, + const int total, + const ReportingType type) { + INVESTIGATOR_HISTOGRAM_CUSTOM_COUNTS("Signin.CookieJar.SignedInCount", type, + signed_in, 1, 10, 10); + INVESTIGATOR_HISTOGRAM_CUSTOM_COUNTS("Signin.CookieJar.SignedOutCount", type, + signed_out, 1, 10, 10); + INVESTIGATOR_HISTOGRAM_CUSTOM_COUNTS("Signin.CookieJar.TotalCount", type, + total, 1, 10, 10); +} + +void LogAccountRelation(const AccountRelation relation, + const ReportingType type) { + INVESTIGATOR_HISTOGRAM_ENUMERATION( + "Signin.CookieJar.ChromeAccountRelation", type, + static_cast<int>(relation), + static_cast<int>(AccountRelation::HISTOGRAM_COUNT)); +} + +void LogIsShared(const bool is_shared, const ReportingType type) { + INVESTIGATOR_HISTOGRAM_BOOLEAN("Signin.IsShared", type, is_shared); +} + } // namespace signin_metrics diff --git a/chromium/components/signin/core/browser/signin_metrics.h b/chromium/components/signin/core/browser/signin_metrics.h index e825fb663c3..f0de9c02a88 100644 --- a/chromium/components/signin/core/browser/signin_metrics.h +++ b/chromium/components/signin/core/browser/signin_metrics.h @@ -137,6 +137,7 @@ enum class AccessPoint : int { ACCESS_POINT_SIGNIN_PROMO, ACCESS_POINT_RECENT_TABS, ACCESS_POINT_UNKNOWN, // This should never have been used to get signin URL. + ACCESS_POINT_PASSWORD_BUBBLE, ACCESS_POINT_MAX, // This must be last. }; @@ -204,7 +205,7 @@ enum CrossDevicePromoInitialized { // histogram, which records the state of the AccountReconcilor when GAIA returns // a specific response. enum AccountReconcilorState { - // The AccountReconcilor has finished running ans is up-to-date. + // The AccountReconcilor has finished running ans is up to date. ACCOUNT_RECONCILOR_OK, // The AccountReconcilor is running and gathering information. ACCOUNT_RECONCILOR_RUNNING, @@ -243,6 +244,42 @@ enum class SignoutDelete : int { IGNORE_METRIC, }; +// This is the relationship between the account used to sign into chrome, and +// the account(s) used to sign into the content area/cookie jar. This enum +// gets messy because we're trying to capture quite a few things, if there was +// a match or not, how many accounts were in the cookie jar, and what state +// those cookie jar accounts were in (signed out vs signed in). Note that it's +// not possible to have the same account multiple times in the cookie jar. +enum class AccountRelation : int { + // No signed in or out accounts in the content area. Cannot have a match. + EMPTY_COOKIE_JAR = 0, + // The cookie jar contains only a single signed out account that matches. + NO_SIGNED_IN_SINGLE_SIGNED_OUT_MATCH, + // The cookie jar contains only signed out accounts, one of which matches. + NO_SIGNED_IN_ONE_OF_SIGNED_OUT_MATCH, + // The cookie jar contains one or more signed out accounts, none match. + NO_SIGNED_IN_WITH_SIGNED_OUT_NO_MATCH, + // The cookie jar contains only a single signed in account that matches. + SINGLE_SIGNED_IN_MATCH_NO_SIGNED_OUT, + // There's only one signed in account which matches, and there are one or + // more signed out accounts. + SINGLE_SINGED_IN_MATCH_WITH_SIGNED_OUT, + // There's more than one signed in account, one of which matches. There + // could be any configuration of signed out accounts. + ONE_OF_SIGNED_IN_MATCH_ANY_SIGNED_OUT, + // There's one or more signed in accounts, none of which match. However + // there is a match in the signed out accounts. + WITH_SIGNED_IN_ONE_OF_SIGNED_OUT_MATCH, + // There's one or more signed in accounts and any configuration of signed + // out accounts. However, none of the accounts match. + WITH_SIGNED_IN_NO_MATCH, + // Always the last enumerated type. + HISTOGRAM_COUNT, +}; + +// Different types of reporting. This is used as a histogram suffix. +enum class ReportingType { PERIODIC, ON_CHANGE }; + // Tracks the access point of sign in. void LogSigninAccessPointStarted(AccessPoint access_point); void LogSigninAccessPointCompleted(AccessPoint access_point); @@ -313,6 +350,62 @@ void LogAccountReconcilorStateOnGaiaResponse(AccountReconcilorState state); // and previous id/emails during a signin. void LogAccountEquality(AccountEquality equality); +// Records the amount of time since the cookie jar was last changed. +void LogCookieJarStableAge(const base::TimeDelta stable_age, + const ReportingType type); + +// Records three counts for the number of accounts in the cookei jar. +void LogCookieJarCounts(const int signed_in, + const int signed_out, + const int total, + const ReportingType type); + +// Records the relation between the account signed into Chrome, and the +// account(s) present in the cookie jar. +void LogAccountRelation(const AccountRelation relation, + const ReportingType type); + +// Records if the best guess is that this profile is currently shared or not +// between multiple users. +void LogIsShared(const bool is_shared, const ReportingType type); + +// These intermediate macros are necessary when we may emit to different +// histograms from the same logical place in the code. The base histogram macros +// expand in a way that can only work for a single histogram name, so these +// allow a single place in the code to fan out for multiple names. +#define INVESTIGATOR_HISTOGRAM_CUSTOM_COUNTS(name, type, sample, min, max, \ + bucket_count) \ + switch (type) { \ + case ReportingType::PERIODIC: \ + UMA_HISTOGRAM_CUSTOM_COUNTS(name "_Periodic", sample, min, max, \ + bucket_count); \ + break; \ + case ReportingType::ON_CHANGE: \ + UMA_HISTOGRAM_CUSTOM_COUNTS(name "_OnChange", sample, min, max, \ + bucket_count); \ + break; \ + } + +#define INVESTIGATOR_HISTOGRAM_BOOLEAN(name, type, sample) \ + switch (type) { \ + case ReportingType::PERIODIC: \ + UMA_HISTOGRAM_BOOLEAN(name "_Periodic", sample); \ + break; \ + case ReportingType::ON_CHANGE: \ + UMA_HISTOGRAM_BOOLEAN(name "_OnChange", sample); \ + break; \ + } + +#define INVESTIGATOR_HISTOGRAM_ENUMERATION(name, type, sample, boundary_value) \ + switch (type) { \ + case ReportingType::PERIODIC: \ + UMA_HISTOGRAM_ENUMERATION(name "_Periodic", sample, boundary_value); \ + break; \ + case ReportingType::ON_CHANGE: \ + UMA_HISTOGRAM_ENUMERATION(name "_OnChange", sample, boundary_value); \ + break; \ + } + } // namespace signin_metrics #endif // COMPONENTS_SIGNIN_CORE_BROWSER_SIGNIN_METRICS_H_ diff --git a/chromium/components/signin/core/browser/signin_status_metrics_provider.cc b/chromium/components/signin/core/browser/signin_status_metrics_provider.cc index e656df1a0a5..04d54fa8a7f 100644 --- a/chromium/components/signin/core/browser/signin_status_metrics_provider.cc +++ b/chromium/components/signin/core/browser/signin_status_metrics_provider.cc @@ -13,27 +13,6 @@ #include "base/threading/thread_task_runner_handle.h" #include "components/signin/core/browser/signin_manager.h" -namespace { - -// The event of calling function ComputeCurrentSigninStatus and the errors -// occurred during the function execution. -enum ComputeSigninStatus { - ENTERED_COMPUTE_SIGNIN_STATUS, - ERROR_NO_PROFILE_FOUND, - NO_BROWSER_OPENED, - USER_SIGNIN_WHEN_STATUS_UNKNOWN, - USER_SIGNOUT_WHEN_STATUS_UNKNOWN, - TRY_TO_OVERRIDE_ERROR_STATUS, - COMPUTE_SIGNIN_STATUS_MAX, -}; - -void RecordComputeSigninStatusHistogram(ComputeSigninStatus status) { - UMA_HISTOGRAM_ENUMERATION("UMA.ComputeCurrentSigninStatus", status, - COMPUTE_SIGNIN_STATUS_MAX); -} - -} // namespace - SigninStatusMetricsProvider::SigninStatusMetricsProvider( std::unique_ptr<SigninStatusMetricsProviderDelegate> delegate, bool is_test) @@ -104,7 +83,6 @@ void SigninStatusMetricsProvider::GoogleSigninSucceeded( // There should have at least one browser opened if the user can sign in, so // signin_status_ value should not be unknown. UpdateSigninStatus(ERROR_GETTING_SIGNIN_STATUS); - RecordComputeSigninStatusHistogram(USER_SIGNIN_WHEN_STATUS_UNKNOWN); } } @@ -117,7 +95,6 @@ void SigninStatusMetricsProvider::GoogleSignedOut(const std::string& account_id, // There should have at least one browser opened if the user can sign out, // so signin_status_ value should not be unknown. UpdateSigninStatus(ERROR_GETTING_SIGNIN_STATUS); - RecordComputeSigninStatusHistogram(USER_SIGNOUT_WHEN_STATUS_UNKNOWN); } } @@ -154,17 +131,12 @@ void SigninStatusMetricsProvider::UpdateInitialSigninStatus( } void SigninStatusMetricsProvider::ComputeCurrentSigninStatus() { - RecordComputeSigninStatusHistogram(ENTERED_COMPUTE_SIGNIN_STATUS); - AccountsStatus accounts_status = delegate_->GetStatusOfAllAccounts(); if (accounts_status.num_accounts == 0) { - // This should not happen. If it does, record it in histogram. - RecordComputeSigninStatusHistogram(ERROR_NO_PROFILE_FOUND); UpdateSigninStatus(ERROR_GETTING_SIGNIN_STATUS); } else if (accounts_status.num_opened_accounts == 0) { // The code indicates that Chrome is running in the background but no // browser window is opened. - RecordComputeSigninStatusHistogram(NO_BROWSER_OPENED); UpdateSigninStatus(UNKNOWN_SIGNIN_STATUS); } else { UpdateInitialSigninStatus(accounts_status.num_opened_accounts, diff --git a/chromium/components/signin/core/browser/signin_tracker.cc b/chromium/components/signin/core/browser/signin_tracker.cc index 1249fd741b4..b13365dcab3 100644 --- a/chromium/components/signin/core/browser/signin_tracker.cc +++ b/chromium/components/signin/core/browser/signin_tracker.cc @@ -6,18 +6,15 @@ #include "components/signin/core/browser/gaia_cookie_manager_service.h" #include "components/signin/core/browser/profile_oauth2_token_service.h" -#include "components/signin/core/browser/signin_client.h" #include "google_apis/gaia/gaia_constants.h" SigninTracker::SigninTracker(ProfileOAuth2TokenService* token_service, SigninManagerBase* signin_manager, GaiaCookieManagerService* cookie_manager_service, - SigninClient* client, Observer* observer) : token_service_(token_service), signin_manager_(signin_manager), cookie_manager_service_(cookie_manager_service), - client_(client), observer_(observer) { Initialize(); } diff --git a/chromium/components/signin/core/browser/signin_tracker.h b/chromium/components/signin/core/browser/signin_tracker.h index 8592d027572..cabbedf02ca 100644 --- a/chromium/components/signin/core/browser/signin_tracker.h +++ b/chromium/components/signin/core/browser/signin_tracker.h @@ -13,7 +13,6 @@ #include "google_apis/gaia/google_service_auth_error.h" class ProfileOAuth2TokenService; -class SigninClient; // The signin flow logic is spread across several classes with varying // responsibilities: @@ -77,7 +76,6 @@ class SigninTracker : public SigninManagerBase::Observer, SigninTracker(ProfileOAuth2TokenService* token_service, SigninManagerBase* signin_manager, GaiaCookieManagerService* cookie_manager_service, - SigninClient* client, Observer* observer); ~SigninTracker() override; @@ -102,9 +100,6 @@ class SigninTracker : public SigninManagerBase::Observer, SigninManagerBase* signin_manager_; GaiaCookieManagerService* cookie_manager_service_; - // The client associated with this instance. - SigninClient* client_; - // Weak pointer to the observer we call when the signin state changes. Observer* observer_; diff --git a/chromium/components/signin/core/browser/webdata/token_service_table_unittest.cc b/chromium/components/signin/core/browser/webdata/token_service_table_unittest.cc index b6b1e7a910e..fd44a258edf 100644 --- a/chromium/components/signin/core/browser/webdata/token_service_table_unittest.cc +++ b/chromium/components/signin/core/browser/webdata/token_service_table_unittest.cc @@ -7,7 +7,7 @@ #include "base/strings/string_number_conversions.h" #include "base/time/time.h" #include "build/build_config.h" -#include "components/os_crypt/os_crypt.h" +#include "components/os_crypt/os_crypt_mocker.h" #include "components/signin/core/browser/webdata/token_service_table.h" #include "components/webdata/common/web_database.h" #include "testing/gtest/include/gtest/gtest.h" @@ -21,9 +21,7 @@ class TokenServiceTableTest : public testing::Test { protected: void SetUp() override { -#if defined(OS_MACOSX) - OSCrypt::UseMockKeychain(true); -#endif + OSCryptMocker::SetUpWithSingleton(); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); file_ = temp_dir_.path().AppendASCII("TestWebDatabase"); @@ -33,6 +31,8 @@ class TokenServiceTableTest : public testing::Test { ASSERT_EQ(sql::INIT_OK, db_->Init(file_)); } + void TearDown() override { OSCryptMocker::TearDown(); } + base::FilePath file_; base::ScopedTempDir temp_dir_; std::unique_ptr<TokenServiceTable> table_; diff --git a/chromium/components/signin/core/common/profile_management_switches.cc b/chromium/components/signin/core/common/profile_management_switches.cc index a49776224f2..e66596b2484 100644 --- a/chromium/components/signin/core/common/profile_management_switches.cc +++ b/chromium/components/signin/core/common/profile_management_switches.cc @@ -140,6 +140,10 @@ bool IsMaterialDesignUserManager() { return base::FeatureList::IsEnabled(switches::kMaterialDesignUserManager); } +bool IsMaterialDesignUserMenu() { + return base::FeatureList::IsEnabled(switches::kMaterialDesignUserMenu); +} + void EnableNewProfileManagementForTesting(base::CommandLine* command_line) { command_line->AppendSwitch(switches::kEnableNewProfileManagement); DCHECK(!command_line->HasSwitch(switches::kDisableNewProfileManagement)); diff --git a/chromium/components/signin/core/common/profile_management_switches.h b/chromium/components/signin/core/common/profile_management_switches.h index a2f1f1c6360..98f47093dbd 100644 --- a/chromium/components/signin/core/common/profile_management_switches.h +++ b/chromium/components/signin/core/common/profile_management_switches.h @@ -38,6 +38,9 @@ bool UsePasswordSeparatedSigninFlow(); // Whether the new Material Design User Manager should be displayed. bool IsMaterialDesignUserManager(); +// Whether the material design user menu should be displayed. +bool IsMaterialDesignUserMenu(); + // Called in tests to force enabling different modes. void EnableNewProfileManagementForTesting(base::CommandLine* command_line); void EnableAccountConsistencyForTesting(base::CommandLine* command_line); diff --git a/chromium/components/signin/core/common/signin_pref_names.cc b/chromium/components/signin/core/common/signin_pref_names.cc index dcf3b595480..564cd6184b7 100644 --- a/chromium/components/signin/core/common/signin_pref_names.cc +++ b/chromium/components/signin/core/common/signin_pref_names.cc @@ -14,6 +14,18 @@ const char kAccountIdMigrationState[] = "account_id_migration_state"; // Boolean identifying whether reverse auto-login is enabled. const char kAutologinEnabled[] = "autologin.enabled"; +// A hash of the GAIA accounts present in the content area. Order does not +// affect the hash, but signed in/out status will. Stored as the Base64 string. +const char kGaiaCookieHash[] = "gaia_cookie.hash"; + +// The last time that kGaiaCookieHash was last updated. Stored as a double that +// should be converted into base::Time. +const char kGaiaCookieChangedTime[] = "gaia_cookie.changed_time"; + +// The last time that periodic reporting occured, to allow us to report as close +// to once per intended interval as possible, through restarts. Stored as a +// double that should be converted into base::Time. +const char kGaiaCookiePeriodicReportTime[] = "gaia_cookie.periodic_report_time"; // The profile's hosted domain; empty if unset; // AccountTrackerService::kNoHostedDomainFound if there is none. diff --git a/chromium/components/signin/core/common/signin_pref_names.h b/chromium/components/signin/core/common/signin_pref_names.h index 226830ea638..8306d9dadf9 100644 --- a/chromium/components/signin/core/common/signin_pref_names.h +++ b/chromium/components/signin/core/common/signin_pref_names.h @@ -9,6 +9,9 @@ namespace prefs { extern const char kAccountIdMigrationState[]; extern const char kAutologinEnabled[]; +extern const char kGaiaCookieHash[]; +extern const char kGaiaCookieChangedTime[]; +extern const char kGaiaCookiePeriodicReportTime[]; extern const char kGoogleServicesAccountId[]; extern const char kGoogleServicesHostedDomain[]; extern const char kGoogleServicesLastAccountId[]; diff --git a/chromium/components/signin/core/common/signin_switches.cc b/chromium/components/signin/core/common/signin_switches.cc index 504d75ca23d..70eddfecdaf 100644 --- a/chromium/components/signin/core/common/signin_switches.cc +++ b/chromium/components/signin/core/common/signin_switches.cc @@ -39,10 +39,15 @@ const base::Feature kMaterialDesignUserManager { "MaterialDesignUserManager", base::FEATURE_DISABLED_BY_DEFAULT }; +// Enables or disables the material design desktop user menu. +const base::Feature kMaterialDesignUserMenu { + "MaterialDesignUserMenu", base::FEATURE_DISABLED_BY_DEFAULT +}; + // Enables or disables the new password separated sign in flow in a tab modal // dialog. const base::Feature kUsePasswordSeparatedSigninFlow { - "UsePasswordSeparatedSigninFlow", base::FEATURE_DISABLED_BY_DEFAULT + "UsePasswordSeparatedSigninFlow", base::FEATURE_ENABLED_BY_DEFAULT }; } // namespace switches diff --git a/chromium/components/signin/core/common/signin_switches.h b/chromium/components/signin/core/common/signin_switches.h index 3407c820283..70e5ef66128 100644 --- a/chromium/components/signin/core/common/signin_switches.h +++ b/chromium/components/signin/core/common/signin_switches.h @@ -26,6 +26,7 @@ extern const char kExtensionsMultiAccount[]; extern const char kGoogleProfileInfo[]; extern const base::Feature kMaterialDesignUserManager; +extern const base::Feature kMaterialDesignUserMenu; extern const base::Feature kUsePasswordSeparatedSigninFlow; } // namespace switches diff --git a/chromium/components/signin/ios/browser/account_consistency_service.h b/chromium/components/signin/ios/browser/account_consistency_service.h index 45d650bef02..39b80d42842 100644 --- a/chromium/components/signin/ios/browser/account_consistency_service.h +++ b/chromium/components/signin/ios/browser/account_consistency_service.h @@ -138,6 +138,7 @@ class AccountConsistencyService : public KeyedService, const GoogleServiceAuthError& error) override; void OnGaiaAccountsInCookieUpdated( const std::vector<gaia::ListedAccount>& accounts, + const std::vector<gaia::ListedAccount>& signed_out_accounts, const GoogleServiceAuthError& error) override; // SigninManagerBase::Observer implementation. diff --git a/chromium/components/signin/ios/browser/account_consistency_service.mm b/chromium/components/signin/ios/browser/account_consistency_service.mm index 52634f85e63..1ce0e2e2cc4 100644 --- a/chromium/components/signin/ios/browser/account_consistency_service.mm +++ b/chromium/components/signin/ios/browser/account_consistency_service.mm @@ -452,6 +452,7 @@ void AccountConsistencyService::OnAddAccountToCookieCompleted( void AccountConsistencyService::OnGaiaAccountsInCookieUpdated( const std::vector<gaia::ListedAccount>& accounts, + const std::vector<gaia::ListedAccount>& signed_out_accounts, const GoogleServiceAuthError& error) { AddChromeConnectedCookies(); } diff --git a/chromium/components/signin/ios/browser/fake_profile_oauth2_token_service_ios_provider.h b/chromium/components/signin/ios/browser/fake_profile_oauth2_token_service_ios_provider.h index 84e8f8ebd29..bd3287cc52e 100644 --- a/chromium/components/signin/ios/browser/fake_profile_oauth2_token_service_ios_provider.h +++ b/chromium/components/signin/ios/browser/fake_profile_oauth2_token_service_ios_provider.h @@ -27,8 +27,6 @@ class FakeProfileOAuth2TokenServiceIOSProvider const std::set<std::string>& scopes, const AccessTokenCallback& callback) override; std::vector<AccountInfo> GetAllAccounts() const override; - AccountInfo GetAccountInfoForEmail(const std::string& email) const override; - AccountInfo GetAccountInfoForGaia(const std::string& gaia) const override; AuthenticationErrorCategory GetAuthenticationErrorCategory( const std::string& gaia_id, NSError* error) const override; diff --git a/chromium/components/signin/ios/browser/fake_profile_oauth2_token_service_ios_provider.mm b/chromium/components/signin/ios/browser/fake_profile_oauth2_token_service_ios_provider.mm index a7337f4a212..763fa823542 100644 --- a/chromium/components/signin/ios/browser/fake_profile_oauth2_token_service_ios_provider.mm +++ b/chromium/components/signin/ios/browser/fake_profile_oauth2_token_service_ios_provider.mm @@ -32,26 +32,6 @@ FakeProfileOAuth2TokenServiceIOSProvider::GetAllAccounts() const { } ProfileOAuth2TokenServiceIOSProvider::AccountInfo -FakeProfileOAuth2TokenServiceIOSProvider::GetAccountInfoForEmail( - const std::string& email) const { - for (const auto& account : accounts_) { - if (account.email == email) - return account; - } - return ProfileOAuth2TokenServiceIOSProvider::AccountInfo(); -} - -ProfileOAuth2TokenServiceIOSProvider::AccountInfo -FakeProfileOAuth2TokenServiceIOSProvider::GetAccountInfoForGaia( - const std::string& gaia) const { - for (const auto& account : accounts_) { - if (account.gaia == gaia) - return account; - } - return ProfileOAuth2TokenServiceIOSProvider::AccountInfo(); -} - -ProfileOAuth2TokenServiceIOSProvider::AccountInfo FakeProfileOAuth2TokenServiceIOSProvider::AddAccount(const std::string& gaia, const std::string& email) { ProfileOAuth2TokenServiceIOSProvider::AccountInfo account; diff --git a/chromium/components/signin/ios/browser/profile_oauth2_token_service_ios_provider.h b/chromium/components/signin/ios/browser/profile_oauth2_token_service_ios_provider.h index ed8b3d6a4b4..4bd55c36d44 100644 --- a/chromium/components/signin/ios/browser/profile_oauth2_token_service_ios_provider.h +++ b/chromium/components/signin/ios/browser/profile_oauth2_token_service_ios_provider.h @@ -56,14 +56,6 @@ class ProfileOAuth2TokenServiceIOSProvider { // Returns the ids of all accounts. virtual std::vector<AccountInfo> GetAllAccounts() const; - // Returns the account info composed of a GAIA id and email corresponding to - // email |email|. - virtual AccountInfo GetAccountInfoForEmail(const std::string& email) const; - - // Returns the account info composed of a GAIA id and email corresponding to - // GAIA id |gaia|. - virtual AccountInfo GetAccountInfoForGaia(const std::string& gaia) const; - // Starts fetching an access token for the account with id |gaia_id| with // the given |scopes|. Once the token is obtained, |callback| is called. virtual void GetAccessToken(const std::string& gaia_id, diff --git a/chromium/components/signin/ios/browser/profile_oauth2_token_service_ios_provider.mm b/chromium/components/signin/ios/browser/profile_oauth2_token_service_ios_provider.mm index 372bfacc86b..4144c6f2ec7 100644 --- a/chromium/components/signin/ios/browser/profile_oauth2_token_service_ios_provider.mm +++ b/chromium/components/signin/ios/browser/profile_oauth2_token_service_ios_provider.mm @@ -9,18 +9,6 @@ ProfileOAuth2TokenServiceIOSProvider::GetAllAccounts() const { return std::vector<ProfileOAuth2TokenServiceIOSProvider::AccountInfo>(); } -ProfileOAuth2TokenServiceIOSProvider::AccountInfo -ProfileOAuth2TokenServiceIOSProvider::GetAccountInfoForEmail( - const std::string& email) const { - return ProfileOAuth2TokenServiceIOSProvider::AccountInfo(); -} - -ProfileOAuth2TokenServiceIOSProvider::AccountInfo -ProfileOAuth2TokenServiceIOSProvider::GetAccountInfoForGaia( - const std::string& gaia) const { - return ProfileOAuth2TokenServiceIOSProvider::AccountInfo(); -} - void ProfileOAuth2TokenServiceIOSProvider::GetAccessToken( const std::string& gaia_id, const std::string& client_id, |