Skip to content

Commit

Permalink
Revert "Move ExtensionCache to //extensions"
Browse files Browse the repository at this point in the history
This reverts commit 1056c8a.

Reason for revert: Seems to have caused test flakiness with non-empty message
loop. See https://crbug.com/422884.

BUG=422884
TBR=rockot@chromium.org

Review URL: https://codereview.chromium.org/653623002

Cr-Commit-Position: refs/heads/master@{#299294}
  • Loading branch information
mkustermann authored and Commit bot committed Oct 13, 2014
1 parent 02708e2 commit 98ff22a
Show file tree
Hide file tree
Showing 28 changed files with 129 additions and 214 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -178,17 +178,10 @@ void DeviceLocalAccountExternalPolicyLoaderTest::SetForceInstallListPolicy() {
store_.NotifyStoreLoaded();
}

#if defined(OS_CHROMEOS)
// crbug.com/422884
#define MAYBE_CacheNotStarted DISABLED_CacheNotStarted
#else
#define MAYBE_CacheNotStarted CacheNotStarted
#endif

// Verifies that when the cache is not explicitly started, the loader does not
// serve any extensions, even if the force-install list policy is set or a load
// is manually requested.
TEST_F(DeviceLocalAccountExternalPolicyLoaderTest, MAYBE_CacheNotStarted) {
TEST_F(DeviceLocalAccountExternalPolicyLoaderTest, CacheNotStarted) {
// Set the force-install list policy.
SetForceInstallListPolicy();

Expand Down
36 changes: 4 additions & 32 deletions chrome/browser/chromeos/policy/auto_enrollment_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -394,14 +394,7 @@ TEST_F(AutoEnrollmentClientTest, NetworkChangeRetryAfterErrors) {
EXPECT_TRUE(HasCachedDecision());
}

#if defined(OS_CHROMEOS)
// crbug.com/422884
#define MAYBE_CancelAndDeleteSoonWithPendingRequest DISABLED_CancelAndDeleteSoonWithPendingRequest
#else
#define MAYBE_CancelAndDeleteSoonWithPendingRequest CancelAndDeleteSoonWithPendingRequest
#endif

TEST_F(AutoEnrollmentClientTest, MAYBE_CancelAndDeleteSoonWithPendingRequest) {
TEST_F(AutoEnrollmentClientTest, CancelAndDeleteSoonWithPendingRequest) {
MockDeviceManagementJob* job = NULL;
ServerWillReplyAsync(&job);
EXPECT_FALSE(job);
Expand All @@ -422,14 +415,7 @@ TEST_F(AutoEnrollmentClientTest, MAYBE_CancelAndDeleteSoonWithPendingRequest) {
EXPECT_EQ(AUTO_ENROLLMENT_STATE_PENDING, state_);
}

#if defined(OS_CHROMEOS)
// crbug.com/422884
#define MAYBE_NetworkChangedAfterCancelAndDeleteSoon DISABLED_NetworkChangedAfterCancelAndDeleteSoon
#else
#define MAYBE_NetworkChangedAfterCancelAndDeleteSoon
#endif

TEST_F(AutoEnrollmentClientTest, MAYBE_NetworkChangedAfterCancelAndDeleteSoon) {
TEST_F(AutoEnrollmentClientTest, NetworkChangedAfterCancelAndDeleteSoon) {
MockDeviceManagementJob* job = NULL;
ServerWillReplyAsync(&job);
EXPECT_FALSE(job);
Expand Down Expand Up @@ -459,14 +445,7 @@ TEST_F(AutoEnrollmentClientTest, MAYBE_NetworkChangedAfterCancelAndDeleteSoon) {
EXPECT_EQ(AUTO_ENROLLMENT_STATE_PENDING, state_);
}

#if defined(OS_CHROMEOS)
// crbug.com/422884
#define MAYBE_CancelAndDeleteSoonAfterCompletion DISABLED_CancelAndDeleteSoonAfterCompletion
#else
#define MAYBE_CancelAndDeleteSoonAfterCompletion CancelAndDeleteSoonAfterCompletion
#endif

TEST_F(AutoEnrollmentClientTest, MAYBE_CancelAndDeleteSoonAfterCompletion) {
TEST_F(AutoEnrollmentClientTest, CancelAndDeleteSoonAfterCompletion) {
ServerWillReply(-1, true, true);
ServerWillSendState(
"example.com",
Expand All @@ -481,14 +460,7 @@ TEST_F(AutoEnrollmentClientTest, MAYBE_CancelAndDeleteSoonAfterCompletion) {
EXPECT_TRUE(base::MessageLoop::current()->IsIdleForTesting());
}

#if defined(OS_CHROMEOS)
// crbug.com/422884
#define MAYBE_CancelAndDeleteSoonAfterNetworkFailure DISABLED_CancelAndDeleteSoonAfterNetworkFailure
#else
#define MAYBE_CancelAndDeleteSoonAfterNetworkFailure CancelAndDeleteSoonAfterNetworkFailure
#endif

TEST_F(AutoEnrollmentClientTest, MAYBE_CancelAndDeleteSoonAfterNetworkFailure) {
TEST_F(AutoEnrollmentClientTest, CancelAndDeleteSoonAfterNetworkFailure) {
ServerWillFail(DM_STATUS_TEMPORARY_UNAVAILABLE);
client_->Start();
EXPECT_EQ(AUTO_ENROLLMENT_STATE_SERVER_ERROR, state_);
Expand Down
23 changes: 5 additions & 18 deletions chrome/browser/extensions/chrome_extensions_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@
#include "extensions/browser/url_request_util.h"

#if defined(OS_CHROMEOS)
#include "chrome/browser/extensions/updater/extension_cache_impl.h"
#include "chromeos/chromeos_switches.h"
#else
#include "extensions/browser/updater/null_extension_cache.h"
#endif

namespace extensions {
Expand All @@ -54,12 +51,6 @@ ChromeExtensionsBrowserClient::ChromeExtensionsBrowserClient() {
// Only set if it hasn't already been set (e.g. by a test).
if (GetCurrentChannel() == GetDefaultChannel())
SetCurrentChannel(chrome::VersionInfo::GetChannel());

#if defined(OS_CHROMEOS)
extension_cache_.reset(new ExtensionCacheImpl());
#else
extension_cache_.reset(new NullExtensionCache());
#endif
}

ChromeExtensionsBrowserClient::~ChromeExtensionsBrowserClient() {}
Expand Down Expand Up @@ -251,13 +242,6 @@ void ChromeExtensionsBrowserClient::RegisterExtensionFunctions(
extensions::api::GeneratedFunctionRegistry::RegisterAll(registry);
}

scoped_ptr<extensions::RuntimeAPIDelegate>
ChromeExtensionsBrowserClient::CreateRuntimeAPIDelegate(
content::BrowserContext* context) const {
return scoped_ptr<extensions::RuntimeAPIDelegate>(
new ChromeRuntimeAPIDelegate(context));
}

ComponentExtensionResourceManager*
ChromeExtensionsBrowserClient::GetComponentExtensionResourceManager() {
if (!resource_manager_)
Expand All @@ -276,8 +260,11 @@ net::NetLog* ChromeExtensionsBrowserClient::GetNetLog() {
return g_browser_process->net_log();
}

ExtensionCache* ChromeExtensionsBrowserClient::GetExtensionCache() {
return extension_cache_.get();
scoped_ptr<extensions::RuntimeAPIDelegate>
ChromeExtensionsBrowserClient::CreateRuntimeAPIDelegate(
content::BrowserContext* context) const {
return scoped_ptr<extensions::RuntimeAPIDelegate>(
new ChromeRuntimeAPIDelegate(context));
}

} // namespace extensions
3 changes: 0 additions & 3 deletions chrome/browser/extensions/chrome_extensions_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ class ChromeExtensionsBrowserClient : public ExtensionsBrowserClient {
const std::string& event_name,
scoped_ptr<base::ListValue> args) override;
virtual net::NetLog* GetNetLog() override;
virtual ExtensionCache* GetExtensionCache() override;

private:
friend struct base::DefaultLazyInstanceTraits<ChromeExtensionsBrowserClient>;
Expand All @@ -112,8 +111,6 @@ class ChromeExtensionsBrowserClient : public ExtensionsBrowserClient {

scoped_ptr<ChromeComponentExtensionResourceManager> resource_manager_;

scoped_ptr<ExtensionCache> extension_cache_;

DISALLOW_COPY_AND_ASSIGN(ChromeExtensionsBrowserClient);
};

Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/extensions/extension_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include "chrome/browser/extensions/extension_util.h"
#include "chrome/browser/extensions/unpacked_installer.h"
#include "chrome/browser/extensions/updater/extension_cache_fake.h"
#include "chrome/browser/extensions/updater/extension_updater.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/ui/browser.h"
Expand Down Expand Up @@ -133,10 +132,6 @@ void ExtensionBrowserTest::SetUpCommandLine(CommandLine* command_line) {
void ExtensionBrowserTest::SetUpOnMainThread() {
InProcessBrowserTest::SetUpOnMainThread();
observer_.reset(new ExtensionTestNotificationObserver(browser()));
if (extension_service()->updater()) {
extension_service()->updater()->SetExtensionCacheForTesting(
test_extension_cache_.get());
}
}

const Extension* ExtensionBrowserTest::LoadExtension(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ class ExtensionDisabledGlobalErrorTest : public ExtensionBrowserTest {
}

virtual void SetUpOnMainThread() override {
ExtensionBrowserTest::SetUpOnMainThread();
EXPECT_TRUE(scoped_temp_dir_.CreateUniqueTempDir());
service_ = extensions::ExtensionSystem::Get(
browser()->profile())->extension_service();
Expand Down
8 changes: 3 additions & 5 deletions chrome/browser/extensions/extension_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "chrome/browser/extensions/shared_module_service.h"
#include "chrome/browser/extensions/unpacked_installer.h"
#include "chrome/browser/extensions/updater/chrome_extension_downloader_factory.h"
#include "chrome/browser/extensions/updater/extension_cache.h"
#include "chrome/browser/extensions/updater/extension_downloader.h"
#include "chrome/browser/extensions/updater/extension_updater.h"
#include "chrome/browser/google/google_brand.h"
Expand All @@ -67,12 +68,10 @@
#include "extensions/browser/extension_prefs.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/extension_system.h"
#include "extensions/browser/extensions_browser_client.h"
#include "extensions/browser/install_flag.h"
#include "extensions/browser/runtime_data.h"
#include "extensions/browser/uninstall_reason.h"
#include "extensions/browser/update_observer.h"
#include "extensions/browser/updater/extension_cache.h"
#include "extensions/common/extension_messages.h"
#include "extensions/common/extension_urls.h"
#include "extensions/common/feature_switch.h"
Expand Down Expand Up @@ -186,8 +185,7 @@ bool ExtensionService::OnExternalExtensionUpdateUrlFound(

if (Manifest::IsExternalLocation(location)) {
// All extensions that are not user specific can be cached.
extensions::ExtensionsBrowserClient::Get()->GetExtensionCache()
->AllowCaching(id);
extensions::ExtensionCache::GetInstance()->AllowCaching(id);
}

const Extension* extension = GetExtensionById(id, true);
Expand Down Expand Up @@ -309,7 +307,7 @@ ExtensionService::ExtensionService(Profile* profile,
profile->GetPrefs(),
profile,
update_frequency,
extensions::ExtensionsBrowserClient::Get()->GetExtensionCache(),
extensions::ExtensionCache::GetInstance(),
base::Bind(ChromeExtensionDownloaderFactory::CreateForProfile,
profile)));
}
Expand Down
4 changes: 0 additions & 4 deletions chrome/browser/extensions/external_provider_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_service_test_base.h"
#include "chrome/browser/extensions/updater/extension_cache_fake.h"
#include "chrome/browser/extensions/updater/extension_updater.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/extensions/extension_constants.h"
Expand Down Expand Up @@ -63,9 +62,6 @@ class ExternalProviderImplTest : public ExtensionServiceTestBase {
#endif
InitializeExtensionServiceWithUpdater();

service()->updater()->SetExtensionCacheForTesting(
test_extension_cache_.get());

// Don't install default apps. Some of the default apps are downloaded from
// the webstore, ignoring the url we pass to kAppsGalleryUpdateURL, which
// would cause the external updates to never finish install.
Expand Down
84 changes: 84 additions & 0 deletions chrome/browser/extensions/updater/extension_cache.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/extensions/updater/extension_cache.h"

#include "base/memory/singleton.h"

#if defined(OS_CHROMEOS)
#include "chrome/browser/extensions/updater/extension_cache_impl.h"
#endif // OS_CHROMEOS

namespace extensions {
namespace {

#if !defined(OS_CHROMEOS)

// Implementation of ExtensionCache that doesn't cache anything.
// Real cache is used only on Chrome OS other OSes use this null implementation.
class ExtensionCacheNullImpl : public ExtensionCache {
public:
static ExtensionCacheNullImpl* GetInstance() {
return Singleton<ExtensionCacheNullImpl>::get();
}

// Implementation of ExtensionCache.
virtual void Start(const base::Closure& callback) override {
callback.Run();
}

virtual void Shutdown(const base::Closure& callback) override {
callback.Run();
}

virtual void AllowCaching(const std::string& id) override {
}

virtual bool GetExtension(const std::string& id,
base::FilePath* file_path,
std::string* version) override {
return false;
}

virtual void PutExtension(const std::string& id,
const base::FilePath& file_path,
const std::string& version,
const PutExtensionCallback& callback) override {
callback.Run(file_path, true);
}

private:
friend struct DefaultSingletonTraits<ExtensionCacheNullImpl>;

ExtensionCacheNullImpl() {}
virtual ~ExtensionCacheNullImpl() {}
};

#endif // !OS_CHROMEOS

static ExtensionCache* g_extension_cache_override = NULL;

} // namespace

// static
ExtensionCache* ExtensionCache::GetInstance() {
if (g_extension_cache_override) {
return g_extension_cache_override;
} else {
#if defined(OS_CHROMEOS)
return ExtensionCacheImpl::GetInstance();
#else
return ExtensionCacheNullImpl::GetInstance();
#endif
}
}

// static
ExtensionCache* ExtensionCache::SetForTesting(ExtensionCache* cache) {
ExtensionCache* temp = g_extension_cache_override;
g_extension_cache_override = cache;
return temp;
}

} // namespace extensions
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef EXTENSIONS_BROWSER_UPDATER_EXTENSION_CACHE_H_
#define EXTENSIONS_BROWSER_UPDATER_EXTENSION_CACHE_H_
#ifndef CHROME_BROWSER_EXTENSIONS_UPDATER_EXTENSION_CACHE_H_
#define CHROME_BROWSER_EXTENSIONS_UPDATER_EXTENSION_CACHE_H_

#include <string>

Expand All @@ -16,13 +16,13 @@ namespace extensions {
// between multiple users and profiles on the machine.
class ExtensionCache {
public:
// Return global singleton instance of ExtensionCache.
static ExtensionCache* GetInstance();

// Callback that is invoked when the file placed when PutExtension done.
typedef base::Callback<void(const base::FilePath& file_path,
bool file_ownership_passed)> PutExtensionCallback;

ExtensionCache() {}
virtual ~ExtensionCache() {}

// Initialize cache in background. The |callback| is called when cache ready.
// Can be called multiple times. The |callback| can be called immediately if
// cache is ready.
Expand Down Expand Up @@ -54,10 +54,14 @@ class ExtensionCache {
const std::string& version,
const PutExtensionCallback& callback) = 0;

private:
DISALLOW_COPY_AND_ASSIGN(ExtensionCache);
protected:
virtual ~ExtensionCache() {}

// Sets the singleton to the given |cache|. Returns the previous value of
// the singleton. Ownership is not transferred.
static ExtensionCache* SetForTesting(ExtensionCache* cache);
};

} // namespace extensions

#endif // EXTENSIONS_BROWSER_UPDATER_EXTENSION_CACHE_H_
#endif // CHROME_BROWSER_EXTENSIONS_UPDATER_EXTENSION_CACHE_H_
2 changes: 2 additions & 0 deletions chrome/browser/extensions/updater/extension_cache_fake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
namespace extensions {

ExtensionCacheFake::ExtensionCacheFake() {
ExtensionCache::SetForTesting(this);
}

ExtensionCacheFake::~ExtensionCacheFake() {
ExtensionCache::SetForTesting(NULL);
}

void ExtensionCacheFake::Start(const base::Closure& callback) {
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/extensions/updater/extension_cache_fake.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include <set>
#include <string>

#include "extensions/browser/updater/extension_cache.h"
#include "chrome/browser/extensions/updater/extension_cache.h"

namespace extensions {

Expand Down
Loading

0 comments on commit 98ff22a

Please sign in to comment.