Set ThemeService |profile_| in constructor, not Init().

This should fix the crashes in question, which deref |profile_| during
construction.  It also seems more obvious; it's not clear why we should avoid
providing the profile until Init().

This also does a lot of misc. cleanup, e.g. moving member inits into the
declaration.

Bug: 1033456
Change-Id: I44f57c90bb14da4ee116259340b7892a1db6802e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1965899Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#724731}

chrome/browser/themes/theme_service.cc

@@ -319,23 +319,14 @@ const char ThemeService::kDefaultThemeID[] = "";
 
 const char ThemeService::kAutogeneratedThemeID[] = "autogenerated_theme_id";
 
-ThemeService::ThemeService()
-    : ready_(false),
-      rb_(ui::ResourceBundle::GetSharedInstance()),
-      profile_(nullptr),
-      installed_pending_load_id_(kDefaultThemeID),
-      number_of_reinstallers_(0),
-      original_theme_provider_(*this, false, false),
-      incognito_theme_provider_(*this, true, false),
-      default_theme_provider_(*this, false, true) {}
+ThemeService::ThemeService(Profile* profile) : profile_(profile) {}
 
 ThemeService::~ThemeService() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
-void ThemeService::Init(Profile* profile) {
+void ThemeService::Init() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  profile_ = profile;
 
   // TODO(https://crbug.com/953978): Use GetNativeTheme() for all platforms.
   ui::NativeTheme* native_theme = ui::NativeTheme::GetInstanceForNativeUi();
@@ -880,8 +871,11 @@ base::RefCountedMemory* ThemeService::GetRawData(
   base::RefCountedMemory* data = nullptr;
   if (theme_supplier_)
     data = theme_supplier_->GetRawData(id, scale_factor);
-  if (!data)
-    data = rb_.LoadDataResourceBytesForScale(id, ui::SCALE_FACTOR_100P);
+  if (!data) {
+    data =
+        ui::ResourceBundle::GetSharedInstance().LoadDataResourceBytesForScale(
+            id, ui::SCALE_FACTOR_100P);
+  }
 
   return data;
 }
@@ -901,8 +895,10 @@ gfx::Image ThemeService::GetImageNamed(int id, bool incognito) const {
   if (theme_supplier_)
     image = theme_supplier_->GetImageNamed(adjusted_id);
 
-  if (image.IsEmpty())
-    image = rb_.GetNativeImageNamed(adjusted_id);
+  if (image.IsEmpty()) {
+    image = ui::ResourceBundle::GetSharedInstance().GetNativeImageNamed(
+        adjusted_id);
+  }
 
   return image;
 }

chrome/browser/themes/theme_service.h

@@ -47,10 +47,6 @@ namespace theme_service_internal {
 class ThemeServiceTest;
 }
 
-namespace ui {
-class ResourceBundle;
-}
-
 class ThemeService : public content::NotificationObserver,
                      public KeyedService,
                      public ui::NativeThemeObserver {
@@ -77,10 +73,10 @@ class ThemeService : public content::NotificationObserver,
   // Constant ID to use for all autogenerated themes.
   static const char kAutogeneratedThemeID[];
 
-  ThemeService();
+  explicit ThemeService(Profile* profile);
   ~ThemeService() override;
 
-  virtual void Init(Profile* profile);
+  void Init();
 
   // KeyedService:
   void Shutdown() override;
@@ -208,7 +204,7 @@ class ThemeService : public content::NotificationObserver,
   // True if the theme service is ready to be used.
   // TODO(pkotwicz): Add DCHECKS to the theme service's getters once
   // ThemeSource no longer uses the ThemeService when it is not ready.
-  bool ready_;
+  bool ready_ = false;
 
  private:
   // This class implements ui::ThemeProvider on behalf of ThemeService and keeps
@@ -334,7 +330,6 @@ class ThemeService : public content::NotificationObserver,
                                           bool incognito,
                                           bool* has_custom_color) const;
 
-  ui::ResourceBundle& rb_;
   Profile* profile_;
 
   scoped_refptr<CustomThemeSupplier> theme_supplier_;
@@ -344,10 +339,10 @@ class ThemeService : public content::NotificationObserver,
   // never be loaded if the install is due to updating a disabled theme.
   // |pending_install_id_| should be set to |kDefaultThemeID| if there are no
   // recently installed theme extensions
-  std::string installed_pending_load_id_;
+  std::string installed_pending_load_id_ = kDefaultThemeID;
 
   // The number of infobars currently displayed.
-  int number_of_reinstallers_;
+  int number_of_reinstallers_ = 0;
 
   // A cache of already-computed values for COLOR_TOOLBAR_TOP_SEPARATOR, which
   // can be expensive to compute.
@@ -362,9 +357,9 @@ class ThemeService : public content::NotificationObserver,
   std::unique_ptr<ThemeObserver> theme_observer_;
 #endif
 
-  BrowserThemeProvider original_theme_provider_;
-  BrowserThemeProvider incognito_theme_provider_;
-  BrowserThemeProvider default_theme_provider_;
+  BrowserThemeProvider original_theme_provider_{*this, false, false};
+  BrowserThemeProvider incognito_theme_provider_{*this, true, false};
+  BrowserThemeProvider default_theme_provider_{*this, false, true};
 
   SEQUENCE_CHECKER(sequence_checker_);
 

chrome/browser/themes/theme_service_aura_linux.cc

@@ -81,8 +81,6 @@ SystemThemeX11::~SystemThemeX11() {}
 
 }  // namespace
 
-ThemeServiceAuraLinux::ThemeServiceAuraLinux() = default;
-
 ThemeServiceAuraLinux::~ThemeServiceAuraLinux() = default;
 
 bool ThemeServiceAuraLinux::ShouldInitWithSystemTheme() const {

chrome/browser/themes/theme_service_aura_linux.h

@@ -14,7 +14,7 @@ class Profile;
 // provides the native Linux theme.
 class ThemeServiceAuraLinux : public ThemeService {
  public:
-  ThemeServiceAuraLinux();
+  using ThemeService::ThemeService;
   ~ThemeServiceAuraLinux() override;
 
   // Overridden from ThemeService:

chrome/browser/themes/theme_service_factory.cc

@@ -61,17 +61,16 @@ ThemeServiceFactory::~ThemeServiceFactory() {}
 
 KeyedService* ThemeServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* profile) const {
-  ThemeService* provider = NULL;
 #if defined(OS_WIN)
-  provider = new ThemeServiceWin;
+  using ThemeService = ThemeServiceWin;
 #elif defined(OS_LINUX) && !defined(OS_CHROMEOS)
-  provider = new ThemeServiceAuraLinux;
-#else
-  provider = new ThemeService;
+  using ThemeService = ThemeServiceAuraLinux;
 #endif
-  provider->Init(static_cast<Profile*>(profile));
 
-  return provider;
+  auto provider =
+      std::make_unique<ThemeService>(static_cast<Profile*>(profile));
+  provider->Init();
+  return provider.release();
 }
 
 void ThemeServiceFactory::RegisterProfilePrefs(

chrome/browser/themes/theme_service_win.cc

@@ -25,7 +25,7 @@ SkColor GetDefaultInactiveFrameColor() {
 
 }  // namespace
 
-ThemeServiceWin::ThemeServiceWin() {
+ThemeServiceWin::ThemeServiceWin(Profile* profile) : ThemeService(profile) {
   // This just checks for Windows 8+ instead of calling DwmColorsAllowed()
   // because we want to monitor the frame color even when a custom frame is in
   // use, so that it will be correct if at any time the user switches to the
@@ -40,8 +40,7 @@ ThemeServiceWin::ThemeServiceWin() {
   }
 }
 
-ThemeServiceWin::~ThemeServiceWin() {
-}
+ThemeServiceWin::~ThemeServiceWin() = default;
 
 bool ThemeServiceWin::ShouldUseNativeFrame() const {
   const bool use_native_frame_if_enabled =

chrome/browser/themes/theme_service_win.h

@@ -14,7 +14,7 @@
 // are relevant to earlier versions of Windows.
 class ThemeServiceWin : public ThemeService {
  public:
-  ThemeServiceWin();
+  explicit ThemeServiceWin(Profile* profile);
   ~ThemeServiceWin() override;
 
  private:

chrome/browser/themes/theme_syncable_service_unittest.cc

@@ -59,7 +59,7 @@ const base::FilePath::CharType kExtensionFilePath[] = FILE_PATH_LITERAL("/oo");
 
 class FakeThemeService : public ThemeService {
  public:
-  FakeThemeService() {}
+  FakeThemeService() : ThemeService(nullptr) {}
 
   // ThemeService implementation
   void DoSetTheme(const extensions::Extension* extension,