Index: chrome/browser/themes/theme_service.cc |
diff --git a/chrome/browser/themes/theme_service.cc b/chrome/browser/themes/theme_service.cc |
index 6a9d0e1e40a102972fba8dd3e20467802ff9fe0a..29ce4a01dd41648af47f1bda4aa70a02e0c2c9de 100644 |
--- a/chrome/browser/themes/theme_service.cc |
+++ b/chrome/browser/themes/theme_service.cc |
@@ -17,10 +17,12 @@ |
#include "base/single_thread_task_runner.h" |
#include "base/strings/string_util.h" |
#include "base/strings/utf_string_conversions.h" |
+#include "base/task_scheduler/post_task.h" |
#include "base/threading/thread_task_runner_handle.h" |
#include "build/build_config.h" |
#include "chrome/browser/chrome_notification_types.h" |
#include "chrome/browser/extensions/extension_service.h" |
+#include "chrome/browser/extensions/theme_installed_infobar_delegate.h" |
#include "chrome/browser/profiles/profile.h" |
#include "chrome/browser/themes/browser_theme_pack.h" |
#include "chrome/browser/themes/custom_theme_supplier.h" |
@@ -262,7 +264,7 @@ void ThemeService::Observe(int type, |
case extensions::NOTIFICATION_EXTENSION_ENABLED: { |
const Extension* extension = Details<const Extension>(details).ptr(); |
if (extension->is_theme()) |
- SetTheme(extension); |
+ DoSetTheme(extension, true); |
break; |
} |
default: |
@@ -271,40 +273,18 @@ void ThemeService::Observe(int type, |
} |
void ThemeService::SetTheme(const Extension* extension) { |
+ DoSetTheme(extension, false); |
+} |
+ |
+void ThemeService::RevertToTheme(const Extension* extension) { |
DCHECK(extension->is_theme()); |
ExtensionService* service = |
extensions::ExtensionSystem::Get(profile_)->extension_service(); |
- if (!service->IsExtensionEnabled(extension->id())) { |
- // |extension| is disabled when reverting to the previous theme via an |
- // infobar. |
- service->EnableExtension(extension->id()); |
- // Enabling the extension will call back to SetTheme(). |
- return; |
- } |
- |
- std::string previous_theme_id = GetThemeID(); |
- |
- // Clear our image cache. |
- FreePlatformCaches(); |
- |
- BuildFromExtension(extension); |
- SaveThemeID(extension->id()); |
- |
- NotifyThemeChanged(); |
- base::RecordAction(UserMetricsAction("Themes_Installed")); |
- |
- if (previous_theme_id != kDefaultThemeID && |
- previous_theme_id != extension->id() && |
- service->GetInstalledExtension(previous_theme_id)) { |
- // Do not disable the previous theme if it is already uninstalled. Sending |
- // NOTIFICATION_BROWSER_THEME_CHANGED causes the previous theme to be |
- // uninstalled when the notification causes the remaining infobar to close |
- // and does not open any new infobars. See crbug.com/468280. |
- |
- // Disable the old theme. |
- service->DisableExtension(previous_theme_id, |
- extensions::Extension::DISABLE_USER_ACTION); |
- } |
+ DCHECK(!service->IsExtensionEnabled(extension->id())); |
+ // |extension| is disabled when reverting to the previous theme via an |
+ // infobar. |
+ service->EnableExtension(extension->id()); |
+ // Enabling the extension will call back to SetTheme(). |
} |
void ThemeService::UseDefaultTheme() { |
@@ -349,8 +329,10 @@ void ThemeService::OnInfobarDisplayed() { |
void ThemeService::OnInfobarDestroyed() { |
number_of_infobars_--; |
- if (number_of_infobars_ == 0) |
+ if (number_of_infobars_ == 0 && |
+ !build_extension_task_tracker_.HasTrackedTasks()) { |
RemoveUnusedThemes(false); |
+ } |
} |
void ThemeService::RemoveUnusedThemes(bool ignore_infobars) { |
@@ -375,8 +357,8 @@ void ThemeService::RemoveUnusedThemes(bool ignore_infobars) { |
for (extensions::ExtensionSet::const_iterator it = extensions->begin(); |
it != extensions->end(); ++it) { |
const extensions::Extension* extension = it->get(); |
- if (extension->is_theme() && |
- extension->id() != current_theme) { |
+ if (extension->is_theme() && extension->id() != current_theme && |
+ extension->id() != building_extension_id_) { |
// Only uninstall themes which are not disabled or are disabled with |
// reason DISABLE_USER_ACTION. We cannot blanket uninstall all disabled |
// themes because externally installed themes are initially disabled. |
@@ -578,7 +560,7 @@ void ThemeService::LoadThemePrefs() { |
// If we don't have a file pack, we're updating from an old version. |
base::FilePath path = prefs->GetFilePath(prefs::kCurrentThemePackFilename); |
- if (path != base::FilePath()) { |
+ if (!path.empty()) { |
path = path.Append(chrome::kThemePackFilename); |
SwapThemeSupplier(BrowserThemePack::BuildFromDataPack(path, current_id)); |
if (theme_supplier_) |
@@ -685,6 +667,15 @@ SkColor ThemeService::GetSeparatorColor(SkColor tab_color, |
return SkColorSetA(separator_color, alpha); |
} |
+void ThemeService::DoSetTheme(const Extension* extension, |
+ bool suppress_infobar) { |
+ DCHECK(extension->is_theme()); |
+ DCHECK(extensions::ExtensionSystem::Get(profile_) |
+ ->extension_service() |
+ ->IsExtensionEnabled(extension->id())); |
+ BuildFromExtension(extension, suppress_infobar); |
+} |
+ |
gfx::ImageSkia* ThemeService::GetImageSkiaNamed(int id, bool incognito) const { |
gfx::Image image = GetImageNamed(id, incognito); |
if (image.IsEmpty()) |
@@ -785,10 +776,6 @@ void ThemeService::OnExtensionServiceReady() { |
// be recreated from the extension. |
MigrateTheme(); |
set_ready(); |
- |
- // Send notification in case anyone requested data and cached it when the |
- // theme service was not ready yet. |
- NotifyThemeChanged(); |
} |
#if BUILDFLAG(ENABLE_EXTENSIONS) |
@@ -806,15 +793,18 @@ void ThemeService::OnExtensionServiceReady() { |
} |
void ThemeService::MigrateTheme() { |
- // TODO(erg): We need to pop up a dialog informing the user that their |
- // theme is being migrated. |
ExtensionService* service = |
extensions::ExtensionSystem::Get(profile_)->extension_service(); |
const Extension* extension = |
service ? service->GetExtensionById(GetThemeID(), false) : nullptr; |
if (extension) { |
DLOG(ERROR) << "Migrating theme"; |
- BuildFromExtension(extension); |
+ // Theme migration is done on the UI thread. Blocking the UI from appearing |
+ // until it's ready is deemed better than showing a blip of the default |
+ // theme. |
+ scoped_refptr<BrowserThemePack> pack(new BrowserThemePack); |
+ BrowserThemePack::BuildFromExtension(extension, pack); |
+ OnThemeBuiltFromExtension(extension->id(), pack, true); |
base::RecordAction(UserMetricsAction("Themes.Migrated")); |
} else { |
DLOG(ERROR) << "Theme is mysteriously gone."; |
@@ -841,10 +831,26 @@ void ThemeService::SaveThemeID(const std::string& id) { |
profile_->GetPrefs()->SetString(prefs::kCurrentThemeID, id); |
} |
-void ThemeService::BuildFromExtension(const Extension* extension) { |
- scoped_refptr<BrowserThemePack> pack( |
- BrowserThemePack::BuildFromExtension(extension)); |
- if (!pack.get()) { |
+void ThemeService::BuildFromExtension(const Extension* extension, |
+ bool suppress_infobar) { |
+ build_extension_task_tracker_.TryCancelAll(); |
+ building_extension_id_ = extension->id(); |
+ scoped_refptr<BrowserThemePack> pack(new BrowserThemePack); |
+ auto task_runner = base::CreateTaskRunnerWithTraits( |
+ {base::MayBlock(), base::TaskPriority::USER_BLOCKING}); |
+ build_extension_task_tracker_.PostTaskAndReply( |
+ task_runner.get(), FROM_HERE, |
+ base::Bind(&BrowserThemePack::BuildFromExtension, extension, pack), |
+ base::Bind(&ThemeService::OnThemeBuiltFromExtension, |
+ weak_ptr_factory_.GetWeakPtr(), extension->id(), pack, |
+ suppress_infobar)); |
+} |
+ |
+void ThemeService::OnThemeBuiltFromExtension( |
+ const extensions::ExtensionId& extension_id, |
+ scoped_refptr<BrowserThemePack> pack, |
+ bool suppress_infobar) { |
+ if (!pack->is_valid()) { |
// TODO(erg): We've failed to install the theme; perhaps we should tell the |
// user? http://crbug.com/34780 |
LOG(ERROR) << "Could not load theme."; |
@@ -855,6 +861,11 @@ void ThemeService::BuildFromExtension(const Extension* extension) { |
extensions::ExtensionSystem::Get(profile_)->extension_service(); |
if (!service) |
return; |
+ const Extension* extension = extensions::ExtensionRegistry::Get(profile_) |
+ ->enabled_extensions() |
+ .GetByID(extension_id); |
+ if (!extension) |
+ return; |
// Write the packed file to disk. |
service->GetFileTaskRunner()->PostTask( |
@@ -865,6 +876,46 @@ void ThemeService::BuildFromExtension(const Extension* extension) { |
// LoadThemePrefs(). |
SavePackName(extension->path()); |
SwapThemeSupplier(pack); |
+ |
+ if (extension->id() == GetThemeID()) { |
+ // Same old theme, but the theme has changed (migrated). |
+ NotifyThemeChanged(); |
+ return; |
+ } |
+ |
+ const std::string previous_theme_id = GetThemeID(); |
+ const bool previous_using_system_theme = UsingSystemTheme(); |
+ |
+ // Clear our image cache. |
+ FreePlatformCaches(); |
+ SaveThemeID(extension->id()); |
+ NotifyThemeChanged(); |
pkotwicz
2017/06/01 03:21:08
Can this be simplified as:
const std::string prev
Evan Stade
2017/06/01 17:43:02
seems safe to make that change. FreePlatformCaches
|
+ |
+ base::RecordAction(UserMetricsAction("Themes_Installed")); |
+ |
+ bool previous_theme_available = |
+ previous_theme_id == kDefaultThemeID || previous_using_system_theme; |
pkotwicz
2017/06/01 03:21:08
The system theme uses kDefaultThemeID
Evan Stade
2017/06/01 17:43:02
pretty sure this is necessary for GTK to work
pkotwicz
2017/06/01 19:09:05
That's untrue.
ThemeServiceAuraX11::UseSystemThem
Evan Stade
2017/06/01 23:58:54
ok, thanks for pointing this out.
|
+ if (previous_theme_id != kDefaultThemeID && |
pkotwicz
2017/06/01 03:21:08
Can
previous_theme_id != kDefaultThemeId
be simp
Evan Stade
2017/06/01 17:43:02
perhaps, but that reads really strangely IMO, beca
pkotwicz
2017/06/01 19:09:05
Fair enough
|
+ previous_theme_id != extension_id && |
pkotwicz
2017/06/01 03:21:09
I think that the following is always true
previou
Evan Stade
2017/06/01 17:43:02
Done.
|
+ service->GetInstalledExtension(previous_theme_id)) { |
+ // Do not disable the previous theme if it is already uninstalled. Sending |
+ // NOTIFICATION_BROWSER_THEME_CHANGED causes the previous theme to be |
+ // uninstalled when the notification causes the remaining infobar to close |
+ // and does not open any new infobars. See crbug.com/468280. |
+ |
+ // Disable the old theme. |
+ service->DisableExtension(previous_theme_id, |
+ extensions::Extension::DISABLE_USER_ACTION); |
+ |
+ previous_theme_available = true; |
+ } |
+ |
+ // Offer to revert to the old theme. |
+ if (previous_theme_available && !suppress_infobar) { |
+ ThemeInstalledInfoBarDelegate::Create( |
+ extension, profile_, previous_theme_id, previous_using_system_theme); |
+ } |
+ building_extension_id_.clear(); |
} |
#if BUILDFLAG(ENABLE_SUPERVISED_USERS) |