Commit 43cdaa05 authored by Jesse McKenna's avatar Jesse McKenna Committed by Commit Bot

Stop browser_tests creating "Cached Theme.pak"

This change stops browser_tests from creating "Cached Theme.pak" files
in the Chromium source tree. These generated files frequently cause
confusion when they are accidentally added to CLs, and linger in the
Chromium repo.

Background: "Cached Theme.pak" is a preprocessed version of a Chrome
theme. It is created in the theme's directory when a theme is loaded,
to speed up future loading.

The chrome/test/data directory contains seed data for tests, including
some sample themes. When browser tests load these themes, the "Cached
Theme" files are created in the chrome/test/data directory in the
source tree.

To prevent this, this change adds method
ThemeService::DisableThemePackForTesting() for browser tests to call
before loading themes from chrome/test/data.

Bug: 852623
Change-Id: Idc0ccfee80b9e9c26cc0f4d5467dadf35f8a85e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2403588Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Commit-Queue: Jesse McKenna <jessemckenna@google.com>
Cr-Commit-Position: refs/heads/master@{#807957}
parent ae3a7829
......@@ -76,6 +76,7 @@
#include "chrome/browser/extensions/unpacked_installer.h"
#include "chrome/browser/extensions/updater/extension_updater.h"
#include "chrome/browser/policy/profile_policy_connector.h"
#include "chrome/browser/themes/theme_service.h"
#include "chrome/browser/ui/global_error/global_error.h"
#include "chrome/browser/ui/global_error/global_error_service.h"
#include "chrome/browser/ui/global_error/global_error_service_factory.h"
......@@ -2335,6 +2336,10 @@ TEST_F(ExtensionServiceTest, LoadLocalizedTheme) {
base::FilePath extension_path = data_dir().AppendASCII("theme_i18n");
// Don't create "Cached Theme.pak" in the extension directory, so as not to
// modify the source tree.
ThemeService::DisableThemePackForTesting();
UnpackedInstaller::Create(service())->Load(extension_path);
content::RunAllTasksUntilIdle();
EXPECT_EQ(0u, GetErrors().size());
......@@ -2343,13 +2348,6 @@ TEST_F(ExtensionServiceTest, LoadLocalizedTheme) {
const Extension* theme = registry()->enabled_extensions().begin()->get();
EXPECT_EQ("name", theme->name());
EXPECT_EQ("description", theme->description());
// Cleanup the "Cached Theme.pak" file. Ideally, this would be installed in a
// temporary directory, but it automatically installs to the extension's
// directory, and we don't want to copy the whole extension for a unittest.
base::FilePath theme_file = extension_path.Append(chrome::kThemePackFilename);
ASSERT_TRUE(base::PathExists(theme_file));
ASSERT_TRUE(base::DeleteFile(theme_file)); // Not recursive.
}
#if defined(OS_POSIX)
......
......@@ -4,6 +4,7 @@
#include "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/themes/theme_service.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/test/browser_test.h"
......@@ -42,6 +43,10 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest,
// Another part of crbug.com/528026 and related.
IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest,
TestRendererInitializationWithThemesTab) {
// Don't create "Cached Theme.pak" in the extension directory, so as not to
// modify the source tree.
ThemeService::DisableThemePackForTesting();
const Extension* extension = LoadExtensionWithFlags(
test_data_dir_.AppendASCII("theme"), kFlagAllowOldManifestVersions);
ASSERT_TRUE(extension);
......
......@@ -72,9 +72,13 @@ namespace {
// ExtensionService::GarbageCollectExtensions() does something similar.
const int kRemoveUnusedThemesStartupDelay = 30;
bool g_dont_write_theme_pack_for_testing = false;
// Writes the theme pack to disk on a separate thread.
void WritePackToDiskCallback(BrowserThemePack* pack,
const base::FilePath& directory) {
if (g_dont_write_theme_pack_for_testing)
return;
pack->WriteToDisk(directory.Append(chrome::kThemePackFilename));
}
......@@ -469,6 +473,11 @@ SkColor ThemeService::GetAutogeneratedThemeColor() const {
return profile_->GetPrefs()->GetInteger(prefs::kAutogeneratedThemeColor);
}
// static
void ThemeService::DisableThemePackForTesting() {
g_dont_write_theme_pack_for_testing = true;
}
std::unique_ptr<ThemeService::ThemeReinstaller>
ThemeService::BuildReinstallerForCurrentTheme() {
base::OnceClosure reinstall_callback;
......
......@@ -148,6 +148,9 @@ class ThemeService : public KeyedService,
const ThemeHelper& theme_helper_for_testing() const { return theme_helper_; }
// Don't create "Cached Theme.pak" in the extension directory, for testing.
static void DisableThemePackForTesting();
protected:
// Set a custom default theme instead of the normal default theme.
virtual void SetCustomDefaultTheme(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment