Skip to content

Commit

Permalink
Make CookieMonster::Delegate top-level and remove global CookieMonste…
Browse files Browse the repository at this point in the history
…r::EnableFileScheme().

Also, in all modules that depend on content, replaces direct creation of new
net::CookieMonster() with content::CreateInMemoryCookieStore(). This creation
method knows how to respect the --enable-file-cookies flag. Makes webkit
layout tests work.

Relanding parts of https://chromiumcodereview.appspot.com/12546016

TBR=jam,rsleevi,boliu,esprehn
BUG=159193, 331424

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243027 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
ajwong@chromium.org committed Jan 4, 2014
1 parent ec04144 commit 7c4b66b
Show file tree
Hide file tree
Showing 22 changed files with 118 additions and 89 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@
#include "components/autofill/core/browser/credit_card.h"
#include "components/autofill/core/browser/personal_data_manager.h"
#include "components/autofill/core/browser/personal_data_manager_observer.h"
#include "content/public/browser/cookie_store_factory.h"
#include "content/public/browser/dom_storage_context.h"
#include "content/public/browser/local_storage_usage_info.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/test/test_browser_thread.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "net/cookies/cookie_monster.h"
#include "net/ssl/server_bound_cert_service.h"
#include "net/ssl/server_bound_cert_store.h"
#include "net/ssl/ssl_client_cert_type.h"
Expand Down Expand Up @@ -298,7 +298,7 @@ class RemoveSafeBrowsingCookieTester : public RemoveCookieTester {

// Create a cookiemonster that does not have persistant storage, and replace
// the SafeBrowsingService created one with it.
net::CookieStore* monster = new net::CookieMonster(NULL, NULL);
net::CookieStore* monster = content::CreateInMemoryCookieStore(NULL);
sb_service->url_request_context()->GetURLRequestContext()->
set_cookie_store(monster);
SetMonster(monster);
Expand Down
10 changes: 2 additions & 8 deletions chrome/browser/io_thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@
#include "chrome/common/url_constants.h"
#include "components/policy/core/common/policy_service.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/cookie_store_factory.h"
#include "net/base/host_mapping_rules.h"
#include "net/base/net_util.h"
#include "net/base/network_time_notifier.h"
#include "net/base/sdch_manager.h"
#include "net/cert/cert_verifier.h"
#include "net/cert/ct_known_logs.h"
#include "net/cert/ct_verifier.h"
#include "net/cookies/cookie_monster.h"
#include "net/dns/host_cache.h"
#include "net/dns/host_resolver.h"
#include "net/dns/mapped_host_resolver.h"
Expand Down Expand Up @@ -594,7 +594,7 @@ void IOThread::InitAsync() {
globals_->proxy_script_fetcher_proxy_service.reset(
net::ProxyService::CreateDirectWithNetLog(net_log_));
// In-memory cookie store.
globals_->system_cookie_store = new net::CookieMonster(NULL, NULL);
globals_->system_cookie_store = content::CreateInMemoryCookieStore(NULL);
// In-memory server bound cert store.
globals_->system_server_bound_cert_service.reset(
new net::ServerBoundCertService(
Expand Down Expand Up @@ -725,12 +725,6 @@ void IOThread::CleanUp() {
}

void IOThread::InitializeNetworkOptions(const CommandLine& command_line) {
if (command_line.HasSwitch(switches::kEnableFileCookies)) {
// Enable cookie storage for file:// URLs. Must do this before the first
// Profile (and therefore the first CookieMonster) is created.
net::CookieMonster::EnableFileScheme();
}

// Only handle use-spdy command line flags if "spdy.disabled" preference is
// not disabled via policy.
if (is_spdy_disabled_by_policy_) {
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/net/connection_tester.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
#include "base/threading/thread_restrictions.h"
#include "chrome/common/chrome_switches.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/cookie_store_factory.h"
#include "net/base/io_buffer.h"
#include "net/base/net_errors.h"
#include "net/base/net_util.h"
#include "net/base/request_priority.h"
#include "net/cert/cert_verifier.h"
#include "net/cookies/cookie_monster.h"
#include "net/dns/host_resolver.h"
#include "net/http/http_auth_handler_factory.h"
#include "net/http/http_cache.h"
Expand Down Expand Up @@ -132,7 +132,7 @@ class ExperimentURLRequestContext : public net::URLRequestContext {
storage_.set_http_transaction_factory(new net::HttpCache(
network_session.get(), net::HttpCache::DefaultBackend::InMemory(0)));
// In-memory cookie store.
storage_.set_cookie_store(new net::CookieMonster(NULL, NULL));
storage_.set_cookie_store(content::CreateInMemoryCookieStore(NULL));

return net::OK;
}
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/net/connection_tester_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

#include "base/prefs/testing_pref_service.h"
#include "content/public/test/test_browser_thread.h"
#include "content/public/browser/cookie_store_factory.h"
#include "net/cert/mock_cert_verifier.h"
#include "net/cookies/cookie_monster.h"
#include "net/dns/mock_host_resolver.h"
#include "net/ftp/ftp_network_layer.h"
#include "net/http/http_auth_handler_factory.h"
Expand Down Expand Up @@ -148,7 +148,7 @@ class ConnectionTesterTest : public PlatformTest {
http_transaction_factory_.get());
// In-memory cookie store.
proxy_script_fetcher_context_->set_cookie_store(
new net::CookieMonster(NULL, NULL));
content::CreateInMemoryCookieStore(NULL));
}
};

Expand Down
10 changes: 6 additions & 4 deletions chrome/browser/profiles/off_the_record_profile_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/cookie_store_factory.h"
#include "content/public/browser/resource_context.h"
#include "extensions/common/constants.h"
#include "extensions/common/extension.h"
Expand Down Expand Up @@ -200,8 +201,9 @@ void OffTheRecordProfileIOData::InitializeInternal(
set_server_bound_cert_service(server_bound_cert_service);
main_context->set_server_bound_cert_service(server_bound_cert_service);

main_context->set_cookie_store(new net::CookieMonster(
NULL, profile_params->cookie_monster_delegate.get()));
main_context->set_cookie_store(
content::CreateInMemoryCookieStore(
profile_params->cookie_monster_delegate.get()));

net::HttpCache::BackendFactory* main_backend =
net::HttpCache::DefaultBackend::InMemory(0);
Expand Down Expand Up @@ -252,7 +254,7 @@ void OffTheRecordProfileIOData::
// All we care about for extensions is the cookie store. For incognito, we
// use a non-persistent cookie store.
net::CookieMonster* extensions_cookie_store =
new net::CookieMonster(NULL, NULL);
content::CreateInMemoryCookieStore(NULL)->GetCookieMonster();
// Enable cookies for devtools and extension URLs.
const char* schemes[] = {chrome::kChromeDevToolsScheme,
extensions::kExtensionScheme};
Expand Down Expand Up @@ -290,7 +292,7 @@ OffTheRecordProfileIOData::InitializeAppRequestContext(
// Use a separate in-memory cookie store for the app.
// TODO(creis): We should have a cookie delegate for notifying the cookie
// extensions API, but we need to update it to understand isolated apps first.
context->SetCookieStore(new net::CookieMonster(NULL, NULL));
context->SetCookieStore(content::CreateInMemoryCookieStore(NULL));

// Use a separate in-memory cache for the app.
net::HttpCache::BackendFactory* app_backend =
Expand Down
8 changes: 4 additions & 4 deletions chrome/browser/profiles/profile_impl_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -426,8 +426,8 @@ void ProfileImplIOData::InitializeInternal(
net::ServerBoundCertService* server_bound_cert_service = NULL;
if (record_mode || playback_mode) {
// Don't use existing cookies and use an in-memory store.
cookie_store = new net::CookieMonster(
NULL, profile_params->cookie_monster_delegate.get());
cookie_store = content::CreateInMemoryCookieStore(
profile_params->cookie_monster_delegate.get());
// Don't use existing server-bound certs and use an in-memory store.
server_bound_cert_service = new net::ServerBoundCertService(
new net::DefaultServerBoundCertStore(NULL),
Expand Down Expand Up @@ -606,13 +606,13 @@ ProfileImplIOData::InitializeAppRequestContext(

scoped_refptr<net::CookieStore> cookie_store = NULL;
if (partition_descriptor.in_memory) {
cookie_store = new net::CookieMonster(NULL, NULL);
cookie_store = content::CreateInMemoryCookieStore(NULL);
} else if (record_mode || playback_mode) {
// Don't use existing cookies and use an in-memory store.
// TODO(creis): We should have a cookie delegate for notifying the cookie
// extensions API, but we need to update it to understand isolated apps
// first.
cookie_store = new net::CookieMonster(NULL, NULL);
cookie_store = content::CreateInMemoryCookieStore(NULL);
app_http_cache->set_mode(
record_mode ? net::HttpCache::RECORD : net::HttpCache::PLAYBACK);
}
Expand Down
4 changes: 0 additions & 4 deletions chrome/common/chrome_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -638,10 +638,6 @@ const char kEnableExtensionActivityLogTesting[] =
// crbug.com/142458 .
const char kEnableFastUnload[] = "enable-fast-unload";

// By default, cookies are not allowed on file://. They are needed for testing,
// for example page cycler and layout tests. See bug 1157243.
const char kEnableFileCookies[] = "enable-file-cookies";

// Enables Google Now integration.
const char kEnableGoogleNowIntegration[] = "enable-google-now-integration";

Expand Down
1 change: 0 additions & 1 deletion chrome/common/chrome_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ extern const char kEnableEphemeralApps[];
extern const char kEnableExtensionActivityLogging[];
extern const char kEnableExtensionActivityLogTesting[];
extern const char kEnableFastUnload[];
extern const char kEnableFileCookies[];
extern const char kEnableGoogleNowIntegration[];
extern const char kEnableHttp2Draft04[];
extern const char kEnableWebBasedSignin[];
Expand Down
4 changes: 3 additions & 1 deletion chrome/test/base/testing_profile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
#include "components/policy/core/common/policy_service.h"
#include "components/user_prefs/user_prefs.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/cookie_store_factory.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/storage_partition.h"
Expand Down Expand Up @@ -123,7 +124,8 @@ class QuittingHistoryDBTask : public history::HistoryDBTask {
class TestExtensionURLRequestContext : public net::URLRequestContext {
public:
TestExtensionURLRequestContext() {
net::CookieMonster* cookie_monster = new net::CookieMonster(NULL, NULL);
net::CookieMonster* cookie_monster =
content::CreateInMemoryCookieStore(NULL)->GetCookieMonster();
const char* schemes[] = {extensions::kExtensionScheme};
cookie_monster->SetCookieableSchemes(schemes, 1);
set_cookie_store(cookie_monster);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "chrome/test/base/testing_profile.h"
#include "components/autofill/content/browser/wallet/wallet_service_url.h"
#include "components/autofill/content/browser/wallet/wallet_signin_helper_delegate.h"
#include "content/public/browser/cookie_store_factory.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "google_apis/gaia/gaia_constants.h"
#include "google_apis/gaia/gaia_urls.h"
Expand Down Expand Up @@ -125,7 +126,8 @@ TEST_F(WalletSigninHelperTest, PassiveSigninFailedSigninNo) {

TEST_F(WalletSigninHelperTest, GetWalletCookieValueWhenPresent) {
EXPECT_CALL(mock_delegate_, OnDidFetchWalletCookieValue("gdToken"));
net::CookieMonster* cookie_monster = new net::CookieMonster(NULL, NULL);
net::CookieMonster* cookie_monster =
content::CreateInMemoryCookieStore(NULL)->GetCookieMonster();
net::CookieOptions httponly_options;
httponly_options.set_include_httponly();
scoped_ptr<net::CanonicalCookie> cookie(
Expand All @@ -145,7 +147,8 @@ TEST_F(WalletSigninHelperTest, GetWalletCookieValueWhenPresent) {

TEST_F(WalletSigninHelperTest, GetWalletCookieValueWhenMissing) {
EXPECT_CALL(mock_delegate_, OnDidFetchWalletCookieValue(std::string()));
net::CookieMonster* cookie_monster = new net::CookieMonster(NULL, NULL);
net::CookieMonster* cookie_monster =
content::CreateInMemoryCookieStore(NULL)->GetCookieMonster();
net::CookieOptions httponly_options;
httponly_options.set_include_httponly();
scoped_ptr<net::CanonicalCookie> cookie(
Expand Down
34 changes: 33 additions & 1 deletion content/browser/net/sqlite_persistent_cookie_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/basictypes.h"
#include "base/bind.h"
#include "base/callback.h"
#include "base/command_line.h"
#include "base/file_util.h"
#include "base/files/file_path.h"
#include "base/location.h"
Expand All @@ -28,6 +29,7 @@
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/cookie_crypto_delegate.h"
#include "content/public/browser/cookie_store_factory.h"
#include "content/public/common/content_switches.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "net/cookies/canonical_cookie.h"
#include "net/cookies/cookie_constants.h"
Expand Down Expand Up @@ -1264,7 +1266,25 @@ net::CookieStore* CreatePersistentCookieStore(
restore_old_session_cookies,
storage_policy,
crypto_delegate.Pass());
return new net::CookieMonster(persistent_store, cookie_monster_delegate);

net::CookieMonster* cookie_monster =
new net::CookieMonster(persistent_store, cookie_monster_delegate);

// In the case of Android WebView, the cookie store may be created
// before the browser process fully initializes -- certainly before
// the main loop ever runs. In this situation, the CommandLine singleton
// will not have been set up. Android tests do not need file cookies
// so always ignore them here.
//
// TODO(ajwong): Remove the InitializedForCurrentProcess() check
// once http://crbug.com/331424 is resolved.
if (CommandLine::InitializedForCurrentProcess() &&
CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableFileCookies)) {
cookie_monster->SetEnableFileScheme(true);
}

return cookie_monster;
}

net::CookieStore* CreatePersistentCookieStore(
Expand All @@ -1284,4 +1304,16 @@ net::CookieStore* CreatePersistentCookieStore(
crypto_delegate.Pass());
}

net::CookieStore* CreateInMemoryCookieStore(
net::CookieMonster::Delegate* cookie_monster_delegate) {
net::CookieMonster* cookie_monster =
new net::CookieMonster(NULL, cookie_monster_delegate);
if (CommandLine::InitializedForCurrentProcess() &&
CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableFileCookies)) {
cookie_monster->SetEnableFileScheme(true);
}
return cookie_monster;
}

} // namespace content
3 changes: 3 additions & 0 deletions content/public/browser/cookie_store_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ CONTENT_EXPORT net::CookieStore* CreatePersistentCookieStore(
net::CookieMonster::Delegate* cookie_monster_delegate,
scoped_ptr<CookieCryptoDelegate> crypto_delegate);

CONTENT_EXPORT net::CookieStore* CreateInMemoryCookieStore(
net::CookieMonster::Delegate* cookie_monster_delegate);

} // namespace content

#endif // CONTENT_PUBLIC_BROWSER_COOKIE_STORE_FACTORY_H_
4 changes: 4 additions & 0 deletions content/public/common/content_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,10 @@ const char kEnableExperimentalWebPlatformFeatures[] =
// Enable an experimental WebSocket implementation.
const char kEnableExperimentalWebSocket[] = "enable-experimental-websocket";

// By default, cookies are not allowed on file://. They are needed for testing,
// for example page cycler and layout tests. See bug 1157243.
const char kEnableFileCookies[] = "enable-file-cookies";

// Enable the fast text autosizing implementation.
const char kEnableFastTextAutosizing[] = "enable-fast-text-autosizing";

Expand Down
1 change: 1 addition & 0 deletions content/public/common/content_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ CONTENT_EXPORT extern const char kEnableExperimentalCanvasFeatures[];
CONTENT_EXPORT extern const char kEnableExperimentalWebPlatformFeatures[];
CONTENT_EXPORT extern const char kEnableExperimentalWebSocket[];
extern const char kEnableFastTextAutosizing[];
CONTENT_EXPORT extern const char kEnableFileCookies[];
CONTENT_EXPORT extern const char kEnableFixedPositionCreatesStackingContext[];
CONTENT_EXPORT extern const char kEnableGestureTapHighlight[];
extern const char kEnableGpuBenchmarking[];
Expand Down
2 changes: 1 addition & 1 deletion content/shell/app/shell_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ bool ShellMainDelegate::BasicStartupComplete(int* exit_code) {
command_line.AppendSwitch(switches::kDisableDelegatedRenderer);
#endif

net::CookieMonster::EnableFileScheme();
command_line.AppendSwitch(switches::kEnableFileCookies);

// Unless/until WebM files are added to the media layout tests, we need to
// avoid removing MP4/H264/AAC so that layout tests can run on Android.
Expand Down
3 changes: 2 additions & 1 deletion content/shell/browser/shell_url_request_context_getter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/threading/sequenced_worker_pool.h"
#include "base/threading/worker_pool.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/cookie_store_factory.h"
#include "content/public/common/content_switches.h"
#include "content/public/common/url_constants.h"
#include "content/shell/browser/shell_network_delegate.h"
Expand Down Expand Up @@ -101,7 +102,7 @@ net::URLRequestContext* ShellURLRequestContextGetter::GetURLRequestContext() {
url_request_context_->set_network_delegate(network_delegate_.get());
storage_.reset(
new net::URLRequestContextStorage(url_request_context_.get()));
storage_->set_cookie_store(new net::CookieMonster(NULL, NULL));
storage_->set_cookie_store(CreateInMemoryCookieStore(NULL));
storage_->set_server_bound_cert_service(new net::ServerBoundCertService(
new net::DefaultServerBoundCertStore(NULL),
base::WorkerPool::GetTaskRunner(true)));
Expand Down
3 changes: 2 additions & 1 deletion content/test/test_webkit_platform_support.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/metrics/stats_counters.h"
#include "base/path_service.h"
#include "base/strings/utf_string_conversions.h"
#include "content/public/common/content_switches.h"
#include "content/test/mock_webclipboard_impl.h"
#include "content/test/web_gesture_curve_mock.h"
#include "content/test/web_layer_tree_view_impl_for_testing.h"
Expand Down Expand Up @@ -100,7 +101,7 @@ TestWebKitPlatformSupport::TestWebKitPlatformSupport() {
SetThemeEngine(NULL);
#endif

net::CookieMonster::EnableFileScheme();
CommandLine::ForCurrentProcess()->AppendSwitch(switches::kEnableFileCookies);

// Test shell always exposes the GC.
webkit_glue::SetJavaScriptFlags(" --expose-gc");
Expand Down
Loading

0 comments on commit 7c4b66b

Please sign in to comment.