Solve ProfileIOData, Extension, HostContentSettingsMap ordering issue.

chrome/browser/extensions/extension_service.cc

Index: chrome/browser/extensions/extension_service.cc
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index ad71689d7a0f1542241593dd9f90dea75aed5374..3ca55c71a2ac0f65a8cb4364f77e7fcb45e12a3c 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -2047,21 +2047,25 @@ const Extension* ExtensionService::GetPendingExtensionUpdate(
 }
 
 void ExtensionService::RegisterContentSettings(
-    HostContentSettingsMap* host_content_settings_map) {
+    HostContentSettingsMap* host_content_settings_map,
+    Profile* profile) {
+  // Do nothing if extensions are not enabled for this profile.
+  if (!extensions::ContentSettingsService::Get(profile))

[Devlin, 2017/05/05 22:27:56]

The ContentSettingsService is just a BrowserContextKeyedService that's created
with the profile.  Additionally, it's set to be created if it doesn't exist with
the first Get() call.  So this probably isn't the right check.  What exactly are
you trying to determine here?  If it's just whether extensions are enabled...
hmm... we store that data on the ExtensionService, but the whole point is that
that may not exist yet...

[mmenke, 2017/05/05 22:54:34]

It's null for incognito, causing this method to crash.  Except apparently for
signing profiles when switches::kEnableLoginScreenApps is true.  All the test
failures for PS1 were due to this being nullptr.

[Devlin, 2017/05/05 23:00:08]

Ah, okay.  ExtensionService only has a single shared instance across incognito
and regular profiles, so the implementation here before *always* used the
original profile (the one in |profile_| for extension service.  To maintain that
same behavior, we would want to use profile->GetOriginalProfile() in all cases
here.  That would fix the crashing.

[mmenke, 2017/05/08 17:50:35]

I've confirmed that in the original code, we called this for incognito profiles.
 And that while we seem to have a separate ExtensionService for OTR profiles,
the profile_ variable of those extensions services is the original profile, so
I've made the suggested change.  PTAL.

[Devlin, 2017/05/15 16:53:21]

Note: We don't have a separate ExtensionService for OTR profiles.  The exact
ownership is that we have a ExtensionSystem::Shared object which (as the name
implies) is shared between the OTR and regular profiles; the ExtensionService is
owned by this object.  In other words:
Profile* regular_profile = <some profile>;
Profile* otr_profile = regular_profile->GetOffTheRecordProfile();
// This won't crash.
CHECK_EQ(ExtensionSystem::Get(regular_profile)->extension_service(),
         ExtensionSystem::Get(otr_profile)->extension_service());

+    return;
   TRACE_EVENT0("browser,startup", "ExtensionService::RegisterContentSettings");
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   host_content_settings_map->RegisterProvider(
       HostContentSettingsMap::INTERNAL_EXTENSION_PROVIDER,
       std::unique_ptr<content_settings::ObservableProvider>(
-          new content_settings::InternalExtensionProvider(profile_)));
+          new content_settings::InternalExtensionProvider(profile)));

[Devlin, 2017/05/05 22:27:55]

Note: this relies on the ExtensionRegistry, which should be okay (since that can
exist before the extension system).

 
   host_content_settings_map->RegisterProvider(
       HostContentSettingsMap::CUSTOM_EXTENSION_PROVIDER,
       std::unique_ptr<content_settings::ObservableProvider>(
           new content_settings::CustomExtensionProvider(
-              extensions::ContentSettingsService::Get(profile_)
+              extensions::ContentSettingsService::Get(profile)
                   ->content_settings_store(),
-              profile_->GetOriginalProfile() != profile_)));
+              profile->GetOriginalProfile() != profile)));
 }
 
 void ExtensionService::TrackTerminatedExtension(