chrome/browser/extensions/updater/extension_updater_unittest.cc - Issue 456063002: ExtensionUpdater: Abstract ExtensionDownloader creation

Unified Diff: chrome/browser/extensions/updater/extension_updater_unittest.cc

Issue 456063002: ExtensionUpdater: Abstract ExtensionDownloader creation (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: Created 6 years, 4 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: chrome/browser/extensions/updater/extension_updater_unittest.cc

diff --git a/chrome/browser/extensions/updater/extension_updater_unittest.cc b/chrome/browser/extensions/updater/extension_updater_unittest.cc

index ad8ef32abda0afa6a29db85c51743d99de72788c..26210cef7fbad4727f3548e507a8f9527aeb63eb 100644

--- a/chrome/browser/extensions/updater/extension_updater_unittest.cc

+++ b/chrome/browser/extensions/updater/extension_updater_unittest.cc

@@ -127,6 +127,10 @@ int kExpectedLoadFlags =

int kExpectedLoadFlagsForDownloadWithCookies = net::LOAD_DISABLE_CACHE;

+// Fake authentication constants

+const char kFakeAccountId[] = "bobloblaw@lawblog.example.com";

+const char kFakeOAuth2Token[] = "ce n'est pas un jeton";

const ManifestFetchData::PingData kNeverPingedData(

ManifestFetchData::kNeverPinged, ManifestFetchData::kNeverPinged, true);

@@ -280,7 +284,10 @@ int GetAuthUserQueryValue(const GURL& url) {

class MockService : public TestExtensionService {

public:

explicit MockService(TestExtensionPrefs* prefs)

- : prefs_(prefs), pending_extension_manager_(&profile_) {}

+ : prefs_(prefs),

+ pending_extension_manager_(&profile_),

+ downloader_delegate_override_(NULL) {

+ }

virtual ~MockService() {}

@@ -300,6 +307,10 @@ class MockService : public TestExtensionService {

PrefService* pref_service() { return prefs_->pref_service(); }

+ FakeOAuth2TokenService* fake_token_service() {

+ return fake_token_service_.get();

+ }

// Creates test extensions and inserts them into list. The name and

// version are all based on their index. If |update_url| is non-null, it

// will be used as the update_url for each extension.

@@ -323,12 +334,56 @@ class MockService : public TestExtensionService {

}

+ ExtensionDownloader::Factory GetDownloaderFactory() {

+ return base::Bind(&MockService::CreateExtensionDownloader,

+ base::Unretained(this));

+ }

+ ExtensionDownloader::Factory GetAuthenticatedDownloaderFactory() {

+ return base::Bind(&MockService::CreateExtensionDownloaderWithIdentity,

+ base::Unretained(this));

+ }

+ void OverrideDownloaderDelegate(ExtensionDownloaderDelegate* delegate) {

+ downloader_delegate_override_ = delegate;

+ }

protected:

TestExtensionPrefs* const prefs_;

TestingProfile profile_;

PendingExtensionManager pending_extension_manager_;

private:

+ scoped_ptr<ExtensionDownloader> CreateExtensionDownloader(

+ ExtensionDownloaderDelegate* delegate) {

+ return make_scoped_ptr(new ExtensionDownloader(

+ downloader_delegate_override_ ? downloader_delegate_override_

+ : delegate,

+ request_context()));

+ }

+ scoped_ptr<ExtensionDownloader> CreateExtensionDownloaderWithIdentity(

+ ExtensionDownloaderDelegate* delegate) {

+ scoped_ptr<FakeIdentityProvider> fake_identity_provider;

+ fake_token_service_.reset(new FakeOAuth2TokenService());

+ fake_identity_provider.reset(new FakeIdentityProvider(

+ fake_token_service_.get()));

+ fake_identity_provider->LogIn(kFakeAccountId);

+ fake_token_service_->AddAccount(kFakeAccountId);

+ scoped_ptr<ExtensionDownloader> downloader(new ExtensionDownloader(

Yoyo Zhou 2014/08/11 23:19:28 Use CreateExtensionDownloader here.

Ken Rockot(use gerrit already) 2014/08/11 23:48:25 Good catch, thanks. Done.

+ downloader_delegate_override_ ? downloader_delegate_override_

+ : delegate,

+ request_context()));

+ downloader->SetWebstoreIdentityProvider(

+ fake_identity_provider.PassAs<IdentityProvider>());

+ return downloader.Pass();

+ }

+ scoped_ptr<FakeOAuth2TokenService> fake_token_service_;

+ ExtensionDownloaderDelegate* downloader_delegate_override_;

DISALLOW_COPY_AND_ASSIGN(MockService);

};

@@ -377,7 +432,9 @@ void SetupPendingExtensionManagerForTest(

class ServiceForManifestTests : public MockService {

public:

explicit ServiceForManifestTests(TestExtensionPrefs* prefs)

- : MockService(prefs), registry_(ExtensionRegistry::Get(profile())) {}

+ : MockService(prefs),

+ registry_(ExtensionRegistry::Get(profile())) {

+ }

virtual ~ServiceForManifestTests() {}

@@ -571,12 +628,6 @@ class ExtensionUpdaterTest : public testing::Test {

results->list.push_back(result);

}

- void ResetDownloader(ExtensionUpdater* updater,

- ExtensionDownloader* downloader) {

- EXPECT_FALSE(updater->downloader_.get());

- updater->downloader_.reset(downloader);

- }

void StartUpdateCheck(ExtensionDownloader* downloader,

ManifestFetchData* fetch_data) {

downloader->StartUpdateCheck(scoped_ptr<ManifestFetchData>(fetch_data));

@@ -612,7 +663,7 @@ class ExtensionUpdaterTest : public testing::Test {

service.profile(),

60 * 60 * 24,

NULL,

- make_scoped_ptr<IdentityProvider>(NULL));

+ service.GetDownloaderFactory());

updater.Start();

// Tell the update that it's time to do update checks.

@@ -699,7 +750,7 @@ class ExtensionUpdaterTest : public testing::Test {

MockService service(prefs_.get());

MockExtensionDownloaderDelegate delegate;

- ExtensionDownloader downloader(&delegate, service.request_context(), NULL);

+ ExtensionDownloader downloader(&delegate, service.request_context());

ExtensionList extensions;

std::string url(gallery_url);

@@ -741,8 +792,7 @@ class ExtensionUpdaterTest : public testing::Test {

void TestDetermineUpdates() {

TestingProfile profile;

MockExtensionDownloaderDelegate delegate;

- ExtensionDownloader downloader(

- &delegate, profile.GetRequestContext(), NULL);

+ ExtensionDownloader downloader(&delegate, profile.GetRequestContext());

// Check passing an empty list of parse results to DetermineUpdates

ManifestFetchData fetch_data(GURL("http://localhost/foo"), 0);

@@ -785,8 +835,7 @@ class ExtensionUpdaterTest : public testing::Test {

TestingProfile profile;

MockExtensionDownloaderDelegate delegate;

- ExtensionDownloader downloader(

- &delegate, profile.GetRequestContext(), NULL);

+ ExtensionDownloader downloader(&delegate, profile.GetRequestContext());

ManifestFetchData fetch_data(GURL("http://localhost/foo"), 0);

UpdateManifest::Results updates;

@@ -825,7 +874,7 @@ class ExtensionUpdaterTest : public testing::Test {

net::TestURLFetcher* fetcher = NULL;

MockService service(prefs_.get());

MockExtensionDownloaderDelegate delegate;

- ExtensionDownloader downloader(&delegate, service.request_context(), NULL);

+ ExtensionDownloader downloader(&delegate, service.request_context());

downloader.manifests_queue_.set_backoff_policy(&kNoBackoffPolicy);

GURL kUpdateUrl("http://localhost/manifest1");

@@ -965,7 +1014,7 @@ class ExtensionUpdaterTest : public testing::Test {

NotificationsObserver observer;

MockService service(prefs_.get());

MockExtensionDownloaderDelegate delegate;

- ExtensionDownloader downloader(&delegate, service.request_context(), NULL);

+ ExtensionDownloader downloader(&delegate, service.request_context());

downloader.manifests_queue_.set_backoff_policy(&kNoBackoffPolicy);

GURL kUpdateUrl("http://localhost/manifest1");

@@ -1044,13 +1093,12 @@ class ExtensionUpdaterTest : public testing::Test {

service->profile(),

kUpdateFrequencySecs,

NULL,

- make_scoped_ptr<IdentityProvider>(NULL));

- updater.Start();

+ service->GetDownloaderFactory());

MockExtensionDownloaderDelegate delegate;

delegate.DelegateTo(&updater);

- ResetDownloader(

- &updater,

- new ExtensionDownloader(&delegate, service->request_context(), NULL));

+ service->OverrideDownloaderDelegate(&delegate);

+ updater.Start();

+ updater.EnsureDownloaderCreated();

updater.downloader_->extensions_queue_.set_backoff_policy(

&kNoBackoffPolicy);

@@ -1146,44 +1194,30 @@ class ExtensionUpdaterTest : public testing::Test {

bool succeed_with_oauth2,

int valid_authuser,

int max_authuser) {

- const char kFakeAccountId[] = "bobloblaw@lawblog.example.com";

- const char kFakeOAuth2Token[] = "ce n'est pas un jeton";

net::TestURLFetcherFactory factory;

net::TestURLFetcher* fetcher = NULL;

scoped_ptr<ServiceForDownloadTests> service(

new ServiceForDownloadTests(prefs_.get()));

- ExtensionUpdater updater(service.get(),

- service->extension_prefs(),

- service->pref_service(),

- service->profile(),

- kUpdateFrequencySecs,

- NULL,

- make_scoped_ptr<IdentityProvider>(NULL));

+ const ExtensionDownloader::Factory& downloader_factory =

+ enable_oauth2 ? service->GetAuthenticatedDownloaderFactory()

+ : service->GetDownloaderFactory();

+ ExtensionUpdater updater(

+ service.get(),

+ service->extension_prefs(),

+ service->pref_service(),

+ service->profile(),

+ kUpdateFrequencySecs,

+ NULL,

+ downloader_factory);

updater.Start();

- scoped_ptr<FakeOAuth2TokenService> fake_token_service;

- scoped_ptr<FakeIdentityProvider> fake_identity_provider;

- if (enable_oauth2) {

- fake_token_service.reset(new FakeOAuth2TokenService());

- fake_identity_provider.reset(new FakeIdentityProvider(

- fake_token_service.get()));

- fake_identity_provider->LogIn(kFakeAccountId);

- fake_token_service->AddAccount(kFakeAccountId);

- }

- ResetDownloader(

- &updater,

- new ExtensionDownloader(&updater,

- service->request_context(),

- fake_identity_provider.get()));

+ updater.EnsureDownloaderCreated();

updater.downloader_->extensions_queue_.set_backoff_policy(

&kNoBackoffPolicy);

GURL test_url(base::StringPrintf("%s/extension.crx", url_prefix.c_str()));

std::string id = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";

std::string hash;

- Version version("0.0.1");

+ Version version("0.0.1");

std::set<int> requests;

requests.insert(0);

scoped_ptr<ExtensionDownloader::ExtensionFetch> fetch(

@@ -1200,8 +1234,9 @@ class ExtensionUpdaterTest : public testing::Test {

fetcher->set_status(net::URLRequestStatus());

fetcher->set_response_code(403);

fetcher->delegate()->OnURLFetchComplete(fetcher);

- if (fake_token_service) {

- fake_token_service->IssueAllTokensForAccount(

+ if (service->fake_token_service()) {

+ service->fake_token_service()->IssueAllTokensForAccount(

kFakeAccountId, kFakeOAuth2Token, base::Time::Now());

}

RunUntilIdle();

@@ -1353,11 +1388,9 @@ class ExtensionUpdaterTest : public testing::Test {

service.profile(),

kUpdateFrequencySecs,

NULL,

- make_scoped_ptr<IdentityProvider>(NULL));

+ service.GetDownloaderFactory());

updater.Start();

- ResetDownloader(

- &updater,

- new ExtensionDownloader(&updater, service.request_context(), NULL));

+ updater.EnsureDownloaderCreated();

updater.downloader_->extensions_queue_.set_backoff_policy(

&kNoBackoffPolicy);

@@ -1568,7 +1601,7 @@ class ExtensionUpdaterTest : public testing::Test {

service.profile(),

kUpdateFrequencySecs,

NULL,

- make_scoped_ptr<IdentityProvider>(NULL));

+ service.GetDownloaderFactory());

ExtensionUpdater::CheckParams params;

updater.Start();

updater.CheckNow(params);

@@ -1665,11 +1698,9 @@ class ExtensionUpdaterTest : public testing::Test {

service.profile(),

kUpdateFrequencySecs,

NULL,

- make_scoped_ptr<IdentityProvider>(NULL));

+ service.GetDownloaderFactory());

updater.Start();

- ResetDownloader(

- &updater,

- new ExtensionDownloader(&updater, service.request_context(), NULL));

+ updater.EnsureDownloaderCreated();

ManifestFetchData fetch_data(update_url, 0);

const Extension* extension = tmp[0].get();

@@ -1853,13 +1884,9 @@ TEST_F(ExtensionUpdaterTest, TestNonAutoUpdateableLocations) {

service.profile(),

kUpdateFrequencySecs,

NULL,

- make_scoped_ptr<IdentityProvider>(NULL));

+ service.GetDownloaderFactory());

MockExtensionDownloaderDelegate delegate;

- // Set the downloader directly, so that all its events end up in the mock

- // |delegate|.

- ExtensionDownloader* downloader =

- new ExtensionDownloader(&delegate, service.request_context(), NULL);

- ResetDownloader(&updater, downloader);

+ service.OverrideDownloaderDelegate(&delegate);

// Non-internal non-external extensions should be rejected.

ExtensionList extensions;

@@ -1871,7 +1898,8 @@ TEST_F(ExtensionUpdaterTest, TestNonAutoUpdateableLocations) {

// These expectations fail if the delegate's methods are invoked for the

// first extension, which has a non-matching id.

- EXPECT_CALL(delegate, GetUpdateUrlData(updateable_id)).WillOnce(Return(""));

+ EXPECT_CALL(delegate,

+ GetUpdateUrlData(updateable_id)).WillOnce(Return(""));

EXPECT_CALL(delegate, GetPingDataForExtension(updateable_id, _));

service.set_extensions(extensions, ExtensionList());

@@ -1889,13 +1917,9 @@ TEST_F(ExtensionUpdaterTest, TestUpdatingDisabledExtensions) {

service.profile(),

kUpdateFrequencySecs,

NULL,

- make_scoped_ptr<IdentityProvider>(NULL));

+ service.GetDownloaderFactory());

MockExtensionDownloaderDelegate delegate;

- // Set the downloader directly, so that all its events end up in the mock

- // |delegate|.

- ExtensionDownloader* downloader =

- new ExtensionDownloader(&delegate, service.request_context(), NULL);

- ResetDownloader(&updater, downloader);

+ service.OverrideDownloaderDelegate(&delegate);

// Non-internal non-external extensions should be rejected.

ExtensionList enabled_extensions;

@@ -1912,7 +1936,8 @@ TEST_F(ExtensionUpdaterTest, TestUpdatingDisabledExtensions) {

// We expect that both enabled and disabled extensions are auto-updated.

EXPECT_CALL(delegate, GetUpdateUrlData(enabled_id)).WillOnce(Return(""));

EXPECT_CALL(delegate, GetPingDataForExtension(enabled_id, _));

- EXPECT_CALL(delegate, GetUpdateUrlData(disabled_id)).WillOnce(Return(""));

+ EXPECT_CALL(delegate,

+ GetUpdateUrlData(disabled_id)).WillOnce(Return(""));

EXPECT_CALL(delegate, GetPingDataForExtension(disabled_id, _));

service.set_extensions(enabled_extensions, disabled_extensions);

@@ -1926,7 +1951,7 @@ TEST_F(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) {

MockService service(prefs_.get());

MockExtensionDownloaderDelegate delegate;

scoped_ptr<ExtensionDownloader> downloader(

- new ExtensionDownloader(&delegate, service.request_context(), NULL));

+ new ExtensionDownloader(&delegate, service.request_context()));

EXPECT_EQ(0u, ManifestFetchersCount(downloader.get()));

// First, verify that adding valid extensions does invoke the callbacks on

@@ -1957,7 +1982,7 @@ TEST_F(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) {

// Reset the ExtensionDownloader so that it drops the current fetcher.

downloader.reset(

- new ExtensionDownloader(&delegate, service.request_context(), NULL));

+ new ExtensionDownloader(&delegate, service.request_context()));

EXPECT_EQ(0u, ManifestFetchersCount(downloader.get()));

// Extensions with empty update URLs should have a default one

@@ -1978,7 +2003,7 @@ TEST_F(ExtensionUpdaterTest, TestStartUpdateCheckMemory) {

net::TestURLFetcherFactory factory;

MockService service(prefs_.get());

MockExtensionDownloaderDelegate delegate;

- ExtensionDownloader downloader(&delegate, service.request_context(), NULL);

+ ExtensionDownloader downloader(&delegate, service.request_context());

StartUpdateCheck(&downloader, new ManifestFetchData(GURL(), 0));

// This should delete the newly-created ManifestFetchData.

@@ -1998,7 +2023,7 @@ TEST_F(ExtensionUpdaterTest, TestCheckSoon) {

service.profile(),

kUpdateFrequencySecs,

NULL,

- make_scoped_ptr<IdentityProvider>(NULL));

+ service.GetDownloaderFactory());

EXPECT_FALSE(updater.WillCheckSoon());

updater.Start();

EXPECT_FALSE(updater.WillCheckSoon());

« chrome/browser/extensions/updater/extension_updater.cc ('K') | « chrome/browser/extensions/updater/extension_updater.cc ('k') | no next file » | no next file with comments »