Each Profile owns its own SimpleFactoryKey.

Bug: 933784
Change-Id: I3bc03760dea254fa33f8c660689739db65a1ce9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1492791
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Reviewed-by: Xi Han <hanxi@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638295}

chrome/browser/android/cached_image_fetcher/cached_image_fetcher_bridge.cc

@@ -65,7 +65,7 @@ jlong JNI_CachedImageFetcherBridge_Init(
     JNIEnv* j_env,
     const JavaParamRef<jobject>& j_profile) {
   Profile* profile = ProfileAndroid::FromProfileAndroid(j_profile);
-  SimpleFactoryKey* simple_factory_key = Profile::GetSimpleFactoryKey(profile);
+  SimpleFactoryKey* simple_factory_key = profile->GetSimpleFactoryKey();
   base::FilePath file_path =
       CachedImageFetcherServiceFactory::GetCachePath(simple_factory_key)
           .Append(kPathPostfix);

chrome/browser/offline_pages/android/prefetch_test_bridge.cc

@@ -49,10 +49,9 @@ JNI_EXPORT void JNI_PrefetchTestBridge_InsertIntoCachedImageFetcher(
     const JavaParamRef<jbyteArray>& j_image_data) {
   Profile* profile = ProfileManager::GetLastUsedProfile();
   DCHECK(profile);
-  SimpleFactoryKey* simple_factory_key = Profile::GetSimpleFactoryKey(profile);
   image_fetcher::CachedImageFetcherService* service =
       image_fetcher::CachedImageFetcherServiceFactory::GetForKey(
-          simple_factory_key, profile->GetPrefs());
+          profile->GetSimpleFactoryKey(), profile->GetPrefs());
   DCHECK(service);
   scoped_refptr<image_fetcher::ImageCache> cache =
       service->ImageCacheForTesting();

chrome/browser/offline_pages/prefetch/prefetch_service_factory.cc

@@ -106,8 +106,7 @@ KeyedService* PrefetchServiceFactory::BuildServiceInstanceFor(
     suggested_articles_observer = std::make_unique<SuggestedArticlesObserver>();
     thumbnail_fetcher = std::make_unique<ThumbnailFetcherImpl>();
   } else {
-    SimpleFactoryKey* simple_factory_key =
-        Profile::GetSimpleFactoryKey(profile);
+    SimpleFactoryKey* simple_factory_key = profile->GetSimpleFactoryKey();
     image_fetcher::CachedImageFetcherService* image_fetcher_service =
         image_fetcher::CachedImageFetcherServiceFactory::GetForKey(
             simple_factory_key, profile->GetPrefs());

chrome/browser/profiles/off_the_record_profile_impl.cc

@@ -137,7 +137,11 @@ void NotifyOTRProfileDestroyedOnIOThread(void* original_profile,
 }  // namespace
 
 OffTheRecordProfileImpl::OffTheRecordProfileImpl(Profile* real_profile)
-    : profile_(real_profile), start_time_(base::Time::Now()) {
+    : profile_(real_profile),
+      start_time_(base::Time::Now()),
+      key_(
+          std::make_unique<SimpleFactoryKey>(profile_->GetPath(),
+                                             profile_->GetSimpleFactoryKey())) {
   // Must happen before we ask for prefs as prefs needs the connection to the
   // service manager, which is set up in Initialize.
   BrowserContext::Initialize(this, profile_->GetPath());
@@ -148,7 +152,6 @@ OffTheRecordProfileImpl::OffTheRecordProfileImpl(Profile* real_profile)
           ->CreateDelegate());
   // Register on BrowserContext.
   user_prefs::UserPrefs::Set(this, prefs_.get());
-  off_the_record_key_ = profile_->GetOffTheRecordKey();
 }
 
 void OffTheRecordProfileImpl::Init() {
@@ -228,7 +231,7 @@ OffTheRecordProfileImpl::~OffTheRecordProfileImpl() {
   // in the BrowserContextDependencyManager's dependency graph can depend on the
   // ones in the SimpleDependencyManager's graph.
   SimpleDependencyManager::GetInstance()->DestroyKeyedServices(
-      GetOffTheRecordKey());
+      GetSimpleFactoryKey());
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   base::PostTaskWithTraits(
@@ -549,12 +552,9 @@ base::Time OffTheRecordProfileImpl::GetStartTime() const {
   return start_time_;
 }
 
-SimpleFactoryKey* OffTheRecordProfileImpl::GetOriginalKey() const {
-  return off_the_record_key_->original_key();
-}
-
-SimpleFactoryKey* OffTheRecordProfileImpl::GetOffTheRecordKey() const {
-  return off_the_record_key_;
+SimpleFactoryKey* OffTheRecordProfileImpl::GetSimpleFactoryKey() const {
+  DCHECK(key_);
+  return key_.get();
 }
 
 void OffTheRecordProfileImpl::SetExitType(ExitType exit_type) {

chrome/browser/profiles/off_the_record_profile_impl.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_PROFILES_OFF_THE_RECORD_PROFILE_IMPL_H_
 #define CHROME_BROWSER_PROFILES_OFF_THE_RECORD_PROFILE_IMPL_H_
 
+#include <memory>
 #include <string>
 
 #include "base/macros.h"
@@ -77,8 +78,7 @@ class OffTheRecordProfileImpl : public Profile {
       service_manager::mojom::ServiceRequest request) override;
   bool IsSameProfile(Profile* profile) override;
   base::Time GetStartTime() const override;
-  SimpleFactoryKey* GetOriginalKey() const override;
-  SimpleFactoryKey* GetOffTheRecordKey() const override;
+  SimpleFactoryKey* GetSimpleFactoryKey() const override;
   base::FilePath last_selected_directory() override;
   void set_last_selected_directory(const base::FilePath& path) override;
   bool WasCreatedByVersionOrLater(const std::string& version) override;
@@ -154,9 +154,11 @@ class OffTheRecordProfileImpl : public Profile {
   // Time we were started.
   base::Time start_time_;
 
-  base::FilePath last_selected_directory_;
+  // The key to index KeyedService instances created by
+  // SimpleKeyedServiceFactory.
+  std::unique_ptr<SimpleFactoryKey> key_;
 
-  SimpleFactoryKey* off_the_record_key_;
+  base::FilePath last_selected_directory_;
 
   DISALLOW_COPY_AND_ASSIGN(OffTheRecordProfileImpl);
 };

chrome/browser/profiles/profile.cc

@@ -115,13 +115,6 @@ Profile* Profile::FromWebUI(content::WebUI* web_ui) {
   return FromBrowserContext(web_ui->GetWebContents()->GetBrowserContext());
 }
 
-// static
-SimpleFactoryKey* Profile::GetSimpleFactoryKey(Profile* profile) {
-  if (profile->IsOffTheRecord())
-    return profile->GetOffTheRecordKey();
-  return profile->GetOriginalKey();
-}
-
 TestingProfile* Profile::AsTestingProfile() {
   return nullptr;
 }

chrome/browser/profiles/profile.h

@@ -121,9 +121,6 @@ class Profile : public content::BrowserContext {
   // Returns the profile corresponding to the given WebUI.
   static Profile* FromWebUI(content::WebUI* web_ui);
 
-  // Returns the SimpleFactoryKey asscociated with this |profile|.
-  static SimpleFactoryKey* GetSimpleFactoryKey(Profile* profile);
-
   // content::BrowserContext implementation ------------------------------------
 
   // Typesafe upcast.
@@ -225,12 +222,9 @@ class Profile : public content::BrowserContext {
   // the user started chrome.
   virtual base::Time GetStartTime() const = 0;
 
-  // Returns the key used by the original profile to index KeyedService
-  // instances created by a SimpleKeyedServiceFactory.
-  virtual SimpleFactoryKey* GetOriginalKey() const = 0;
-  // Returns the key used by an incognito profile to index KeyedService
-  // instances created by a SimpleKeyedServiceFactory.
-  virtual SimpleFactoryKey* GetOffTheRecordKey() const = 0;
+  // Returns the key used to index KeyedService instances created by a
+  // SimpleKeyedServiceFactory.
+  virtual SimpleFactoryKey* GetSimpleFactoryKey() const = 0;
 
   // Returns the last directory that was chosen for uploading or opening a file.
   virtual base::FilePath last_selected_directory() = 0;

chrome/browser/profiles/profile_impl.cc

@@ -468,6 +468,7 @@ ProfileImpl::ProfileImpl(
       io_data_(this),
       last_session_exit_type_(EXIT_NORMAL),
       start_time_(base::Time::Now()),
+      key_(std::make_unique<SimpleFactoryKey>(GetPath())),
       delegate_(delegate),
       reporting_permissions_checker_factory_(this),
       shared_cors_origin_access_list_(
@@ -476,11 +477,8 @@ ProfileImpl::ProfileImpl(
   DCHECK(!path.empty()) << "Using an empty path will attempt to write "
                         << "profile files to the root directory!";
 
-  // TODO(hanxi): get |key_| and |off_the_record_key_ |from the startup data if
-  // they have been created when this profile is created.
-  key_ = std::make_unique<SimpleFactoryKey>(GetPath());
-  off_the_record_key_ =
-      std::make_unique<SimpleFactoryKey>(GetPath(), key_.get());
+  // TODO(hanxi): get |key_| from the startup data if it has already been
+  // created when this profile is created.
 
 #if defined(OS_CHROMEOS)
   if (!chromeos::ProfileHelper::IsSigninProfile(this) &&
@@ -793,7 +791,7 @@ ProfileImpl::~ProfileImpl() {
   // in the BrowserContextDependencyManager's dependency graph can depend on the
   // ones in the SimpleDependencyManager's graph.
   SimpleDependencyManager::GetInstance()->DestroyKeyedServices(
-      GetOriginalKey());
+      GetSimpleFactoryKey());
 
   // This causes the Preferences file to be written to disk.
   if (prefs_loaded)
@@ -838,14 +836,6 @@ bool ProfileImpl::IsOffTheRecord() const {
   return false;
 }
 
-SimpleFactoryKey* ProfileImpl::GetOriginalKey() const {
-  return key_.get();
-}
-
-SimpleFactoryKey* ProfileImpl::GetOffTheRecordKey() const {
-  return off_the_record_key_.get();
-}
-
 Profile* ProfileImpl::GetOffTheRecordProfile() {
   if (!off_the_record_profile_) {
     std::unique_ptr<Profile> p(CreateOffTheRecordProfile());
@@ -1341,6 +1331,11 @@ base::Time ProfileImpl::GetStartTime() const {
   return start_time_;
 }
 
+SimpleFactoryKey* ProfileImpl::GetSimpleFactoryKey() const {
+  DCHECK(key_);
+  return key_.get();
+}
+
 #if BUILDFLAG(ENABLE_SESSION_SERVICE)
 void ProfileImpl::StopCreateSessionServiceTimer() {
   create_session_service_timer_.Stop();

chrome/browser/profiles/profile_impl.h

@@ -138,8 +138,6 @@ class ProfileImpl : public Profile {
 #if !defined(OS_ANDROID)
   ChromeZoomLevelPrefs* GetZoomLevelPrefs() override;
 #endif
-  SimpleFactoryKey* GetOriginalKey() const override;
-  SimpleFactoryKey* GetOffTheRecordKey() const override;
   PrefService* GetOffTheRecordPrefs() override;
   PrefService* GetReadOnlyOffTheRecordPrefs() override;
   net::URLRequestContextGetter* GetRequestContext() override;
@@ -148,6 +146,7 @@ class ProfileImpl : public Profile {
   scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory() override;
   bool IsSameProfile(Profile* profile) override;
   base::Time GetStartTime() const override;
+  SimpleFactoryKey* GetSimpleFactoryKey() const override;
   base::FilePath last_selected_directory() override;
   void set_last_selected_directory(const base::FilePath& path) override;
   GURL GetHomePage() override;
@@ -252,14 +251,13 @@ class ProfileImpl : public Profile {
 
   std::unique_ptr<Profile> off_the_record_profile_;
 
-  // The keys to index KeyedService instances created by
-  // SimpleKeyedServiceFactory.
-  std::unique_ptr<SimpleFactoryKey> key_;
-  std::unique_ptr<SimpleFactoryKey> off_the_record_key_;
-
   // See GetStartTime for details.
   base::Time start_time_;
 
+  // The key to index KeyedService instances created by
+  // SimpleKeyedServiceFactory.
+  std::unique_ptr<SimpleFactoryKey> key_;
+
 #if defined(OS_CHROMEOS)
   std::unique_ptr<chromeos::Preferences> chromeos_preferences_;
 

chrome/test/base/testing_profile.cc

@@ -307,10 +307,8 @@ TestingProfile::TestingProfile(
       delegate_(delegate),
       profile_name_(profile_name),
       policy_service_(policy_service.release()) {
-  if (parent) {
+  if (parent)
     parent->SetOffTheRecordProfile(std::unique_ptr<Profile>(this));
-    off_the_record_key_ = parent->GetOffTheRecordKey();
-  }
 
   // If no profile path was supplied, create one.
   if (profile_path_.empty()) {
@@ -382,11 +380,12 @@ void TestingProfile::Init() {
                           profile_manager->GetSystemProfilePath());
   }
 
-  if (!IsOffTheRecord()) {
-    owned_key_ = std::make_unique<SimpleFactoryKey>(profile_path_);
-    owned_off_the_record_key_ =
-        std::make_unique<SimpleFactoryKey>(profile_path_, owned_key_.get());
-  }
+  if (IsOffTheRecord())
+    key_ = std::make_unique<SimpleFactoryKey>(
+        original_profile_->GetPath(), original_profile_->GetSimpleFactoryKey());
+  else
+    key_ = std::make_unique<SimpleFactoryKey>(profile_path_);
+
   BrowserContext::Initialize(this, profile_path_);
 
 #if defined(OS_ANDROID)
@@ -483,7 +482,7 @@ void TestingProfile::Init() {
 
   // Prefs for incognito profiles are set in CreateIncognitoPrefService() by
   // simulating ProfileImpl::GetOffTheRecordPrefs().
-  SimpleFactoryKey* key = Profile::GetSimpleFactoryKey(this);
+  SimpleFactoryKey* key = GetSimpleFactoryKey();
   if (!IsOffTheRecord()) {
     DCHECK(!original_profile_);
     user_prefs::PrefRegistrySyncable* pref_registry =
@@ -532,9 +531,7 @@ TestingProfile::~TestingProfile() {
   MaybeSendDestroyedNotification();
 
   browser_context_dependency_manager_->DestroyBrowserContextServices(this);
-
-  SimpleFactoryKey* key = Profile::GetSimpleFactoryKey(this);
-  simple_dependency_manager_->DestroyKeyedServices(key);
+  simple_dependency_manager_->DestroyKeyedServices(GetSimpleFactoryKey());
 
   if (host_content_settings_map_.get())
     host_content_settings_map_->ShutdownOnUIThread();
@@ -898,16 +895,9 @@ base::Time TestingProfile::GetStartTime() const {
   return start_time_;
 }
 
-SimpleFactoryKey* TestingProfile::GetOriginalKey() const {
-  if (IsOffTheRecord())
-    return off_the_record_key_->original_key();
-  return owned_key_.get();
-}
-
-SimpleFactoryKey* TestingProfile::GetOffTheRecordKey() const {
-  if (IsOffTheRecord())
-    return off_the_record_key_;
-  return owned_off_the_record_key_.get();
+SimpleFactoryKey* TestingProfile::GetSimpleFactoryKey() const {
+  DCHECK(key_);
+  return key_.get();
 }
 
 base::FilePath TestingProfile::last_selected_directory() {

chrome/test/base/testing_profile.h

@@ -324,8 +324,7 @@ class TestingProfile : public Profile {
   }
   bool IsSameProfile(Profile* profile) override;
   base::Time GetStartTime() const override;
-  SimpleFactoryKey* GetOriginalKey() const override;
-  SimpleFactoryKey* GetOffTheRecordKey() const override;
+  SimpleFactoryKey* GetSimpleFactoryKey() const override;
   base::FilePath last_selected_directory() override;
   void set_last_selected_directory(const base::FilePath& path) override;
   bool WasCreatedByVersionOrLater(const std::string& version) override;
@@ -369,6 +368,11 @@ class TestingProfile : public Profile {
 
  protected:
   base::Time start_time_;
+
+  // The key to index KeyedService instances created by
+  // SimpleKeyedServiceFactory.
+  std::unique_ptr<SimpleFactoryKey> key_;
+
   std::unique_ptr<sync_preferences::PrefServiceSyncable> prefs_;
   // ref only for right type, lifecycle is managed by prefs_
   sync_preferences::TestingPrefServiceSyncable* testing_prefs_;
@@ -460,14 +464,6 @@ class TestingProfile : public Profile {
 #endif  // defined(OS_CHROMEOS)
 
   std::unique_ptr<policy::PolicyService> policy_service_;
-
-  // The keys to index KeyedService instances created by
-  // SimpleKeyedServiceFactory if this is an original profile.
-  std::unique_ptr<SimpleFactoryKey> owned_key_;
-  std::unique_ptr<SimpleFactoryKey> owned_off_the_record_key_;
-  // The key to index KeyedService instances created by
-  // SimpleKeyedServiceFactory if this is an incognito profile.
-  SimpleFactoryKey* off_the_record_key_ = nullptr;
 };
 
 #endif  // CHROME_TEST_BASE_TESTING_PROFILE_H_