diff options
author | pam@chromium.org <pam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-15 22:09:53 +0000 |
---|---|---|
committer | pam@chromium.org <pam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-15 22:09:53 +0000 |
commit | b6cf240f37f8e953558b1c24bff3568debc76d3f (patch) | |
tree | dad9fa103eb311f2ec030bfbe4f5b8a8a69c5033 | |
parent | 034837b1ac16b04ea7022d142f6a06f86984b41b (diff) | |
download | chromium_src-b6cf240f37f8e953558b1c24bff3568debc76d3f.zip chromium_src-b6cf240f37f8e953558b1c24bff3568debc76d3f.tar.gz chromium_src-b6cf240f37f8e953558b1c24bff3568debc76d3f.tar.bz2 |
Revert 105661 - Revert 105659 - Delay network requests on startup if any webRequest or webNavigation extensions are enabled.
[Relanding 105659. This will re-cause failures in chrome_frame_net_tests, to be fixed in the next CL.]
Add a webRequest extension API permission, used to tell when an extension uses that API and therefore wants to delay startup. Use the "tabs" warning for it.
Also clean up the UserScriptListener, which never released requests individually and so doesn't need to track them individually either, and makes the RequestQueue handle bulk releases by its delegates instead.
BUG=99450
TEST=unit_tests.exe --gtest_filter=NetworkDelayListenerTest.*
Review URL: http://codereview.chromium.org/8205001
TBR=pam@chromium.org
Review URL: http://codereview.chromium.org/8296017
TBR=pam@chromium.org
Review URL: http://codereview.chromium.org/8308003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@105686 0039d316-1c4b-4281-b951-d872f2087c98
23 files changed, 430 insertions, 60 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index be0f372..f4c04d4 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -3835,7 +3835,7 @@ are declared in build/common.gypi. <message name="IDS_EXTENSION_PROMPT_WARNING_CLIPBOARD" desc="Permission string for access to clipboard."> Data you copy and paste </message> - <message name="IDS_EXTENSION_PROMPT_WARNING_TTS_ENGINE" desc="Permission string for access to clipboard."> + <message name="IDS_EXTENSION_PROMPT_WARNING_TTS_ENGINE" desc="Permission string for access to text-to-speech."> Any text spoken using synthesized speech </message> <message name="IDS_EXTENSION_PROMPT_WARNING_CONTENT_SETTINGS" desc="Permission string for access to content settings."> diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc index 6d08b95..073f52d 100644 --- a/chrome/browser/browser_process_impl.cc +++ b/chrome/browser/browser_process_impl.cc @@ -27,6 +27,7 @@ #include "chrome/browser/download/download_request_limiter.h" #include "chrome/browser/extensions/extension_event_router_forwarder.h" #include "chrome/browser/extensions/extension_tab_id_map.h" +#include "chrome/browser/extensions/network_delay_listener.h" #include "chrome/browser/extensions/user_script_listener.h" #include "chrome/browser/first_run/upgrade_util.h" #include "chrome/browser/google/google_url_tracker.h" @@ -762,9 +763,10 @@ void BrowserProcessImpl::CreateResourceDispatcherHost() { resource_dispatcher_host_.get() == NULL); created_resource_dispatcher_host_ = true; - // UserScriptListener will delete itself. + // UserScriptListener and NetworkDelayListener will delete themselves. ResourceQueue::DelegateSet resource_queue_delegates; resource_queue_delegates.insert(new UserScriptListener()); + resource_queue_delegates.insert(new NetworkDelayListener()); resource_dispatcher_host_.reset( new ResourceDispatcherHost(resource_queue_delegates)); diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc index 83b67f3..2a03197 100644 --- a/chrome/browser/extensions/extension_host.cc +++ b/chrome/browser/extensions/extension_host.cc @@ -157,6 +157,22 @@ ExtensionHost::ExtensionHost(const Extension* extension, Source<Profile>(profile_)); } +// This "mock" constructor should only be used by unit tests. +ExtensionHost::ExtensionHost(const Extension* extension, + content::ViewType host_type) + : extension_(extension), + extension_id_(extension->id()), + profile_(NULL), + render_view_host_(NULL), + did_stop_loading_(false), + document_element_available_(false), + url_(GURL()), + ALLOW_THIS_IN_INITIALIZER_LIST( + extension_function_dispatcher_(profile_, this)), + extension_host_type_(host_type), + associated_tab_contents_(NULL) { +} + ExtensionHost::~ExtensionHost() { NotificationService::current()->Notify( chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED, @@ -164,7 +180,9 @@ ExtensionHost::~ExtensionHost() { Details<ExtensionHost>(this)); ProcessCreationQueue::GetInstance()->Remove(this); GetJavaScriptDialogCreatorInstance()->ResetJavaScriptState(this); - render_view_host_->Shutdown(); // deletes render_view_host + // render_view_host_ may be NULL in unit tests. + if (render_view_host_) + render_view_host_->Shutdown(); // deletes render_view_host } void ExtensionHost::CreateView(Browser* browser) { diff --git a/chrome/browser/extensions/extension_host.h b/chrome/browser/extensions/extension_host.h index 84f4b64..3ae64f5 100644 --- a/chrome/browser/extensions/extension_host.h +++ b/chrome/browser/extensions/extension_host.h @@ -195,6 +195,9 @@ class ExtensionHost : public RenderViewHostDelegate, virtual gfx::NativeWindow GetDialogRootWindow() OVERRIDE; protected: + // This should only be used by unit tests. + ExtensionHost(const Extension* extension, content::ViewType host_type); + // Internal functions used to support the CreateNewWidget() method. If a // platform requires plugging into widget creation at a lower level, then a // subclass might want to override these functions, but otherwise they should diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc index f318583..00997a4 100644 --- a/chrome/browser/extensions/extension_prefs.cc +++ b/chrome/browser/extensions/extension_prefs.cc @@ -89,6 +89,12 @@ const char kPrefAllowFileAccess[] = "newAllowFileAccess"; // the old flag and possibly go back to that name. // const char kPrefAllowFileAccessOld[] = "allowFileAccess"; +// A preference indicating that the extension wants to delay network requests +// on browser launch until it indicates it's ready. For example, an extension +// using the webRequest API might want to ensure that no requests are sent +// before it has registered its event handlers. +const char kPrefDelayNetworkRequests[] = "delayNetworkRequests"; + // A preference set by the web store to indicate login information for // purchased apps. const char kWebStoreLogin[] = "extensions.webstore_login"; @@ -877,6 +883,21 @@ bool ExtensionPrefs::HasAllowFileAccessSetting( return ext && ext->HasKey(kPrefAllowFileAccess); } +void ExtensionPrefs::SetDelaysNetworkRequests(const std::string& extension_id, + bool does_delay) { + if (does_delay) { + UpdateExtensionPref(extension_id, kPrefDelayNetworkRequests, + Value::CreateBooleanValue(true)); + } else { + // Remove the pref. + UpdateExtensionPref(extension_id, kPrefDelayNetworkRequests, NULL); + } +} + +bool ExtensionPrefs::DelaysNetworkRequests(const std::string& extension_id) { + return ReadExtensionPrefBoolean(extension_id, kPrefDelayNetworkRequests); +} + ExtensionPrefs::LaunchType ExtensionPrefs::GetLaunchType( const std::string& extension_id, ExtensionPrefs::LaunchType default_pref_value) { @@ -1641,6 +1662,7 @@ void ExtensionPrefs::InitPrefStore(bool extensions_disabled) { value->DeepCopy()); } + // Set content settings. const DictionaryValue* extension_prefs = GetExtensionPref(*ext_id); DCHECK(extension_prefs); ListValue* content_settings = NULL; diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h index a62cbdf..f88812b 100644 --- a/chrome/browser/extensions/extension_prefs.h +++ b/chrome/browser/extensions/extension_prefs.h @@ -239,6 +239,15 @@ class ExtensionPrefs : public ExtensionContentSettingsStore::Observer { void SetAllowFileAccess(const std::string& extension_id, bool allow); bool HasAllowFileAccessSetting(const std::string& extension_id) const; + // Sets the extension preference indicating that an extension wants to delay + // network requests on browser startup. + void SetDelaysNetworkRequests(const std::string& extension_id, + bool does_delay); + + // Returns true if an extension has registered to delay network requests on + // browser startup. + bool DelaysNetworkRequests(const std::string& extension_id); + // Get the launch type preference. If no preference is set, return // |default_pref_value|. LaunchType GetLaunchType(const std::string& extension_id, @@ -253,7 +262,6 @@ class ExtensionPrefs : public ExtensionContentSettingsStore::Observer { const Extension* extension, LaunchType default_pref_value); - // Saves ExtensionInfo for each installed extension with the path to the // version directory and the location. Blacklisted extensions won't be saved // and neither will external extensions the user has explicitly uninstalled. diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index 1fc648d..05dcec7 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -2610,6 +2610,12 @@ void ExtensionService::OnExtensionInstalled( extension_prefs_->SetAllowFileAccess(id, true); } + // If the extension should automatically block network startup (e.g., it uses + // the webRequest API), set the preference. Otherwise clear it, in case the + // extension stopped using a relevant API. + extension_prefs_->SetDelaysNetworkRequests( + extension->id(), extension->ImplicitlyDelaysNetworkStartup()); + NotificationService::current()->Notify( chrome::NOTIFICATION_EXTENSION_INSTALLED, Source<Profile>(profile_), diff --git a/chrome/browser/extensions/extension_webrequest_time_tracker.cc b/chrome/browser/extensions/extension_webrequest_time_tracker.cc index abae501..50d3432 100644 --- a/chrome/browser/extensions/extension_webrequest_time_tracker.cc +++ b/chrome/browser/extensions/extension_webrequest_time_tracker.cc @@ -151,7 +151,7 @@ void ExtensionWebRequestTimeTracker::LogRequestEndTime( if (log.extension_block_durations.empty()) return; - HISTOGRAM_TIMES("Extensions.NetworkDelay", log.block_duration); + UMA_HISTOGRAM_TIMES("Extensions.NetworkDelay", log.block_duration); Analyze(request_id); } diff --git a/chrome/browser/extensions/network_delay_listener.cc b/chrome/browser/extensions/network_delay_listener.cc new file mode 100644 index 0000000..8865c61 --- /dev/null +++ b/chrome/browser/extensions/network_delay_listener.cc @@ -0,0 +1,178 @@ +// Copyright (c) 2011 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/network_delay_listener.h" + +#include "base/bind.h" +#include "base/metrics/histogram.h" +#include "chrome/browser/extensions/extension_host.h" +#include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/common/chrome_notification_types.h" +#include "chrome/common/chrome_view_types.h" +#include "chrome/common/extensions/extension.h" +#include "chrome/common/url_constants.h" +#include "content/browser/browser_thread.h" +#include "content/common/notification_service.h" +#include "net/url_request/url_request.h" + +NetworkDelayListener::NetworkDelayListener() + : resource_queue_(NULL), + extensions_ready_(false), + recorded_startup_delay_(false) { + CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + + registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED, + NotificationService::AllSources()); + registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED, + NotificationService::AllSources()); + registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING, + NotificationService::AllSources()); + registrar_.Add(this, chrome::NOTIFICATION_EXTENSIONS_READY, + NotificationService::AllSources()); + AddRef(); // Will be balanced in Cleanup(). +} + +void NetworkDelayListener::Initialize(ResourceQueue* resource_queue) { + resource_queue_ = resource_queue; +} + +bool NetworkDelayListener::ShouldDelayRequest( + net::URLRequest* request, + const ResourceDispatcherHostRequestInfo& request_info, + const GlobalRequestID& request_id) { + CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + + // Don't block internal URLs. + if (request->url().SchemeIs(chrome::kChromeUIScheme) || + request->url().SchemeIs(chrome::kExtensionScheme) || + request->url().SchemeIs(chrome::kChromeDevToolsScheme)) { + return false; + } + + return !extensions_ready_; +} + +NetworkDelayListener::~NetworkDelayListener() { +} + +void NetworkDelayListener::WillShutdownResourceQueue() { + CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + resource_queue_ = NULL; + + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(&NetworkDelayListener::Cleanup, this)); +} + +void NetworkDelayListener::OnExtensionPending(const std::string& id) { + CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + + DCHECK(pending_extensions_.count(id) == 0); + extensions_ready_ = false; + pending_extensions_.insert(id); + delay_start_times_[id] = base::TimeTicks::Now(); +} + +void NetworkDelayListener::OnExtensionReady(const std::string& id) { + // This may be called multiple times, if an extension finishes loading and is + // then disabled or uninstalled. + CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + + // Quick escape to save work in the common case. + if (pending_extensions_.count(id) == 0) + return; + + UMA_HISTOGRAM_TIMES("Extensions.StartupDelay", + base::TimeTicks::Now() - delay_start_times_[id]); + delay_start_times_.erase(id); + pending_extensions_.erase(id); + + StartDelayedRequestsIfReady(); +} + +void NetworkDelayListener::StartDelayedRequestsIfReady() { + CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + + if (pending_extensions_.empty()) { + extensions_ready_ = true; + if (!recorded_startup_delay_) { + UMA_HISTOGRAM_TIMES("Extensions.StartupDelay_Total", + overall_start_time_.Elapsed()); + recorded_startup_delay_ = true; + } + if (resource_queue_) + resource_queue_->StartDelayedRequests(this); + } +} + +void NetworkDelayListener::Cleanup() { + CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + registrar_.RemoveAll(); + Release(); +} + +void NetworkDelayListener::Observe(int type, + const NotificationSource& source, + const NotificationDetails& details) { + CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + + switch (type) { + case chrome::NOTIFICATION_EXTENSION_LOADED: { + const Extension* extension = Details<const Extension>(details).ptr(); + ExtensionService* service = + Source<Profile>(source).ptr()->GetExtensionService(); + // We only wait for background pages to load. If the extension has no + // background page, ignore it. + if (service->extension_prefs()->DelaysNetworkRequests(extension->id()) && + !extension->background_url().is_empty()) { + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&NetworkDelayListener::OnExtensionPending, + this, extension->id())); + } + break; + } + case chrome::NOTIFICATION_EXTENSION_UNLOADED: { + const Extension* extension = + Details<UnloadedExtensionInfo>(details)->extension; + ExtensionService* service = + Source<Profile>(source).ptr()->GetExtensionService(); + if (service->extension_prefs()->DelaysNetworkRequests(extension->id())) { + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&NetworkDelayListener::OnExtensionReady, + this, extension->id())); + } + break; + } + case chrome::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING: { + const ExtensionHost* eh = Details<ExtensionHost>(details).ptr(); + if (eh->extension_host_type() != + chrome::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) + return; + + const Extension* extension = eh->extension(); + ExtensionService* service = + Source<Profile>(source).ptr()->GetExtensionService(); + if (service->extension_prefs()->DelaysNetworkRequests(extension->id())) { + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&NetworkDelayListener::OnExtensionReady, + this, extension->id())); + } + break; + } + + case chrome::NOTIFICATION_EXTENSIONS_READY: { + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&NetworkDelayListener::StartDelayedRequestsIfReady, this)); + break; + } + + default: + NOTREACHED(); + } +} diff --git a/chrome/browser/extensions/network_delay_listener.h b/chrome/browser/extensions/network_delay_listener.h new file mode 100644 index 0000000..45784d2 --- /dev/null +++ b/chrome/browser/extensions/network_delay_listener.h @@ -0,0 +1,103 @@ +// Copyright (c) 2011 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. + +#ifndef CHROME_BROWSER_EXTENSIONS_NETWORK_DELAY_LISTENER_H_ +#define CHROME_BROWSER_EXTENSIONS_NETWORK_DELAY_LISTENER_H_ +#pragma once + +#include <map> +#include <set> + +#include "base/compiler_specific.h" +#include "base/memory/ref_counted.h" +#include "base/perftimer.h" +#include "base/time.h" +#include "content/browser/renderer_host/resource_queue.h" +#include "content/common/notification_observer.h" +#include "content/common/notification_registrar.h" + +namespace net { +class URLRequest; +} // namespace net + +struct GlobalRequestID; + +// This class handles delaying of resource loads that depend on unloaded +// extensions. For each request that comes in, we check if all extensions are +// ready for it to be loaded; if not, we delay the request. +// +// This class lives mostly on the IO thread. It listens on the UI thread for +// updates to loaded extensions. It will delete itself on the UI thread after +// WillShutdownResourceQueue is called (on the IO thread). +class NetworkDelayListener + : public base::RefCountedThreadSafe<NetworkDelayListener>, + public ResourceQueueDelegate, + public NotificationObserver { + public: + NetworkDelayListener(); + + private: + // ResourceQueueDelegate: + virtual void Initialize(ResourceQueue* resource_queue) OVERRIDE; + virtual bool ShouldDelayRequest( + net::URLRequest* request, + const ResourceDispatcherHostRequestInfo& request_info, + const GlobalRequestID& request_id) OVERRIDE; + virtual void WillShutdownResourceQueue() OVERRIDE; + + friend class base::RefCountedThreadSafe<NetworkDelayListener>; + + virtual ~NetworkDelayListener(); + + // Called when an extension that wants to delay network requests is loading. + void OnExtensionPending(const std::string& id); + + // Called when an extension that wants to delay network requests is ready. + void OnExtensionReady(const std::string& id); + + // If there are no more extensions pending, tell the network queue to resume + // processing requests. + void StartDelayedRequestsIfReady(); + + // Cleanup on UI thread. + void Cleanup(); + + ResourceQueue* resource_queue_; + + // TODO(mpcomplete, pamg): the rest of this stuff should really be + // per-profile, but the complexity doesn't seem worth it at this point. + + // True if the extensions are ready for network requests to proceed. In + // practice this means that the background pages of any pending extensions + // have been run. + bool extensions_ready_; + + // Which extension IDs have registered to delay network requests on startup, + // but are not yet ready for them to resume. + std::set<std::string> pending_extensions_; + + // Used to measure how long each extension has taken to become ready. + std::map<std::string, base::TimeTicks> delay_start_times_; + + // Used to measure total time between the first delay request and resuming + // network requests. + PerfTimer overall_start_time_; + + // Whether we've already calculated total startup time, so we don't corrupt + // the data with entries from installing or enabling single extensions. + bool recorded_startup_delay_; + + // --- UI thread: + + // NotificationObserver + virtual void Observe(int type, + const NotificationSource& source, + const NotificationDetails& details) OVERRIDE; + + NotificationRegistrar registrar_; + + DISALLOW_COPY_AND_ASSIGN(NetworkDelayListener); +}; + +#endif // CHROME_BROWSER_EXTENSIONS_NETWORK_DELAY_LISTENER_H_ diff --git a/chrome/browser/extensions/user_script_listener.cc b/chrome/browser/extensions/user_script_listener.cc index 84a2cc6..267c0b3 100644 --- a/chrome/browser/extensions/user_script_listener.cc +++ b/chrome/browser/extensions/user_script_listener.cc @@ -11,7 +11,6 @@ #include "chrome/common/extensions/extension.h" #include "chrome/common/extensions/url_pattern.h" #include "content/browser/browser_thread.h" -#include "content/browser/renderer_host/global_request_id.h" #include "content/browser/renderer_host/resource_dispatcher_host_request_info.h" #include "content/common/notification_service.h" #include "net/url_request/url_request.h" @@ -30,7 +29,7 @@ struct UserScriptListener::ProfileData { UserScriptListener::UserScriptListener() : resource_queue_(NULL), user_scripts_ready_(false) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED, NotificationService::AllSources()); @@ -51,7 +50,7 @@ bool UserScriptListener::ShouldDelayRequest( net::URLRequest* request, const ResourceDispatcherHostRequestInfo& request_info, const GlobalRequestID& request_id) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); // If it's a frame load, then we need to check the URL against the list of // user scripts to see if we need to wait. @@ -74,7 +73,6 @@ bool UserScriptListener::ShouldDelayRequest( if ((*it).MatchesURL(request->url())) { // One of the user scripts wants to inject into this request, but the // script isn't ready yet. Delay the request. - delayed_request_ids_.push_front(request_id); return true; } } @@ -84,7 +82,7 @@ bool UserScriptListener::ShouldDelayRequest( } void UserScriptListener::WillShutdownResourceQueue() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); resource_queue_ = NULL; BrowserThread::PostTask( @@ -96,7 +94,7 @@ UserScriptListener::~UserScriptListener() { } void UserScriptListener::CheckIfAllUserScriptsReady() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); bool was_ready = user_scripts_ready_; user_scripts_ready_ = true; @@ -107,14 +105,8 @@ void UserScriptListener::CheckIfAllUserScriptsReady() { } if (user_scripts_ready_ && !was_ready) { - if (resource_queue_) { - for (DelayedRequests::iterator it = delayed_request_ids_.begin(); - it != delayed_request_ids_.end(); ++it) { - resource_queue_->StartDelayedRequest(this, *it); - } - } - - delayed_request_ids_.clear(); + if (resource_queue_) + resource_queue_->StartDelayedRequests(this); } } @@ -126,7 +118,7 @@ void UserScriptListener::UserScriptsReady(void* profile_id) { } void UserScriptListener::ProfileDestroyed(void* profile_id) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); profile_data_.erase(profile_id); // We may have deleted the only profile we were waiting on. @@ -148,21 +140,21 @@ void UserScriptListener::AppendNewURLPatterns(void* profile_id, void UserScriptListener::ReplaceURLPatterns(void* profile_id, const URLPatterns& patterns) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); ProfileData& data = profile_data_[profile_id]; data.url_patterns = patterns; } void UserScriptListener::Cleanup() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); registrar_.RemoveAll(); Release(); } void UserScriptListener::CollectURLPatterns(const Extension* extension, URLPatterns* patterns) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); const UserScriptList& scripts = extension->content_scripts(); for (UserScriptList::const_iterator iter = scripts.begin(); @@ -176,7 +168,7 @@ void UserScriptListener::CollectURLPatterns(const Extension* extension, void UserScriptListener::Observe(int type, const NotificationSource& source, const NotificationDetails& details) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); switch (type) { case chrome::NOTIFICATION_EXTENSION_LOADED: { diff --git a/chrome/browser/extensions/user_script_listener.h b/chrome/browser/extensions/user_script_listener.h index 728fde8..1d33c74 100644 --- a/chrome/browser/extensions/user_script_listener.h +++ b/chrome/browser/extensions/user_script_listener.h @@ -37,6 +37,7 @@ class UserScriptListener public: UserScriptListener(); + private: // ResourceQueueDelegate: virtual void Initialize(ResourceQueue* resource_queue) OVERRIDE; virtual bool ShouldDelayRequest( @@ -45,7 +46,6 @@ class UserScriptListener const GlobalRequestID& request_id) OVERRIDE; virtual void WillShutdownResourceQueue() OVERRIDE; - private: friend class base::RefCountedThreadSafe<UserScriptListener>; typedef std::list<URLPattern> URLPatterns; @@ -75,11 +75,6 @@ class UserScriptListener ResourceQueue* resource_queue_; - // A list of every request that we delayed. Will be flushed when user scripts - // are ready. - typedef std::list<GlobalRequestID> DelayedRequests; - DelayedRequests delayed_request_ids_; - // True if all user scripts from all profiles are ready. bool user_scripts_ready_; diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index acbc48a..47a9265 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1208,6 +1208,8 @@ 'browser/extensions/in_memory_extension_settings_storage.h', 'browser/extensions/key_identifier_conversion_views.cc', 'browser/extensions/key_identifier_conversion_views.h', + 'browser/extensions/network_delay_listener.cc', + 'browser/extensions/network_delay_listener.h', 'browser/extensions/pack_extension_job.cc', 'browser/extensions/pack_extension_job.h', 'browser/extensions/pending_extension_info.cc', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 8cd0133..f367eb0 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1315,6 +1315,7 @@ 'browser/extensions/image_loading_tracker_unittest.cc', 'browser/extensions/in_memory_extension_settings_storage_unittest.cc', 'browser/extensions/key_identifier_conversion_views_unittest.cc', + 'browser/extensions/network_delay_listener_unittest.cc', 'browser/extensions/sandboxed_extension_unpacker_unittest.cc', 'browser/extensions/user_script_listener_unittest.cc', 'browser/extensions/user_script_master_unittest.cc', diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc index 94112ba..8d80814 100644 --- a/chrome/common/extensions/extension.cc +++ b/chrome/common/extensions/extension.cc @@ -2835,6 +2835,13 @@ bool Extension::ShowConfigureContextMenus() const { return location() != Extension::COMPONENT; } +bool Extension::ImplicitlyDelaysNetworkStartup() const { + // Network requests should be deferred until any extensions that might want + // to observe or modify them are loaded. + return HasAPIPermission(ExtensionAPIPermission::kWebNavigation) || + HasAPIPermission(ExtensionAPIPermission::kWebRequest); +} + bool Extension::IsDisallowedExperimentalPermission( ExtensionAPIPermission::ID permission) const { return permission == ExtensionAPIPermission::kExperimental && diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h index 4eaf58b..bdf493f 100644 --- a/chrome/common/extensions/extension.h +++ b/chrome/common/extensions/extension.h @@ -422,6 +422,11 @@ class Extension : public base::RefCountedThreadSafe<Extension> { // Whether context menu should be shown for page and browser actions. bool ShowConfigureContextMenus() const; + // Whether network requests should be delayed on browser startup until the + // extension's background page has loaded, even if the extension doesn't + // explicitly request a delay. + bool ImplicitlyDelaysNetworkStartup() const; + // Returns the Homepage URL for this extension. If homepage_url was not // specified in the manifest, this returns the Google Gallery URL. For // third-party extensions, this returns a blank GURL. diff --git a/chrome/common/extensions/extension_permission_set.cc b/chrome/common/extensions/extension_permission_set.cc index fd74af3..f68b58e 100644 --- a/chrome/common/extensions/extension_permission_set.cc +++ b/chrome/common/extensions/extension_permission_set.cc @@ -313,9 +313,11 @@ ExtensionPermissionsInfo::ExtensionPermissionsInfo() IDS_EXTENSION_PROMPT_WARNING_TTS_ENGINE, ExtensionPermissionMessage::kTtsEngine, none); RegisterPermission( - ExtensionAPIPermission::kWebNavigation, "webNavigation", - IDS_EXTENSION_PROMPT_WARNING_TABS, - ExtensionPermissionMessage::kTabs, none); + ExtensionAPIPermission::kWebNavigation, "webNavigation", 0, + ExtensionPermissionMessage::kNone, none); + RegisterPermission( + ExtensionAPIPermission::kWebRequest, "webRequest", 0, + ExtensionPermissionMessage::kNone, none); RegisterPermission( ExtensionAPIPermission::kWebSocketProxyPrivate, "webSocketProxyPrivate", 0, diff --git a/chrome/common/extensions/extension_permission_set.h b/chrome/common/extensions/extension_permission_set.h index 7250174..d7a92cd 100644 --- a/chrome/common/extensions/extension_permission_set.h +++ b/chrome/common/extensions/extension_permission_set.h @@ -122,6 +122,7 @@ class ExtensionAPIPermission { kTtsEngine, kUnlimitedStorage, kWebNavigation, + kWebRequest, kWebSocketProxyPrivate, kWebstorePrivate, kEnumBoundary diff --git a/chrome/common/extensions/extension_permission_set_unittest.cc b/chrome/common/extensions/extension_permission_set_unittest.cc index 5392217..57826cc 100644 --- a/chrome/common/extensions/extension_permission_set_unittest.cc +++ b/chrome/common/extensions/extension_permission_set_unittest.cc @@ -621,8 +621,11 @@ TEST(ExtensionPermissionSetTest, PermissionMessages) { // permissions. skip.insert(ExtensionAPIPermission::kCookie); - // The proxy permission is warned as part of host permission checks. + // The proxy, webNavigation, and webRequest permissions are warned as part of + // host permission checks. skip.insert(ExtensionAPIPermission::kProxy); + skip.insert(ExtensionAPIPermission::kWebNavigation); + skip.insert(ExtensionAPIPermission::kWebRequest); // This permission requires explicit user action (context menu handler) // so we won't prompt for it for now. diff --git a/content/browser/renderer_host/resource_queue.cc b/content/browser/renderer_host/resource_queue.cc index 59d7078..681c8ac 100644 --- a/content/browser/renderer_host/resource_queue.cc +++ b/content/browser/renderer_host/resource_queue.cc @@ -75,19 +75,23 @@ void ResourceQueue::RemoveRequest(const GlobalRequestID& request_id) { requests_.erase(request_id); } -void ResourceQueue::StartDelayedRequest(ResourceQueueDelegate* delegate, - const GlobalRequestID& request_id) { +void ResourceQueue::StartDelayedRequests(ResourceQueueDelegate* delegate) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); DCHECK(!shutdown_); - DCHECK(ContainsKey(interested_delegates_, request_id)); - DCHECK(ContainsKey(interested_delegates_[request_id], delegate)); - interested_delegates_[request_id].erase(delegate); - if (interested_delegates_[request_id].empty()) { - interested_delegates_.erase(request_id); + for (RequestMap::iterator i = requests_.begin(); i != requests_.end(); ++i) { + GlobalRequestID request_id = i->first; + // Ignore requests that this delegate never asked to delay. + if (!ContainsKey(interested_delegates_, request_id) || + !ContainsKey(interested_delegates_[request_id], delegate)) { + continue; + } + interested_delegates_[request_id].erase(delegate); - if (ContainsKey(requests_, request_id)) { - net::URLRequest* request = requests_[request_id]; + // If no more delegates want a delay, start the request. + if (interested_delegates_[request_id].empty()) { + interested_delegates_.erase(request_id); + net::URLRequest* request = i->second; // The request shouldn't have started (SUCCESS is the initial state). DCHECK_EQ(net::URLRequestStatus::SUCCESS, request->status().status()); request->Start(); diff --git a/content/browser/renderer_host/resource_queue.h b/content/browser/renderer_host/resource_queue.h index d8cecec..1119335 100644 --- a/content/browser/renderer_host/resource_queue.h +++ b/content/browser/renderer_host/resource_queue.h @@ -28,8 +28,7 @@ class CONTENT_EXPORT ResourceQueueDelegate { virtual void Initialize(ResourceQueue* resource_queue) = 0; // Should return true if it wants the |request| to not be started at this - // point. To start the delayed request, ResourceQueue::StartDelayedRequest - // should be used. + // point. Use ResourceQueue::StartDelayedRequests to restart requests. virtual bool ShouldDelayRequest( net::URLRequest* request, const ResourceDispatcherHostRequestInfo& request_info, @@ -74,11 +73,10 @@ class CONTENT_EXPORT ResourceQueue { // |request_id| is no longer valid. void RemoveRequest(const GlobalRequestID& request_id); - // A delegate should call StartDelayedRequest when it wants to allow the - // request to start. If it was the last delegate that demanded the request - // to be delayed, the request will be started. - void StartDelayedRequest(ResourceQueueDelegate* delegate, - const GlobalRequestID& request_id); + // A delegate should call StartDelayedRequests when it wants to allow all + // its delayed requests to start. If it was the last delegate that required + // a request to be delayed, that request will be started. + void StartDelayedRequests(ResourceQueueDelegate* delegate); private: typedef std::map<GlobalRequestID, net::URLRequest*> RequestMap; diff --git a/content/browser/renderer_host/resource_queue_unittest.cc b/content/browser/renderer_host/resource_queue_unittest.cc index eb7bd88..c743573 100644 --- a/content/browser/renderer_host/resource_queue_unittest.cc +++ b/content/browser/renderer_host/resource_queue_unittest.cc @@ -78,7 +78,6 @@ class AlwaysDelayingDelegate : public ResourceQueueDelegate { net::URLRequest* request, const ResourceDispatcherHostRequestInfo& request_info, const GlobalRequestID& request_id) { - delayed_requests_.push_back(request_id); return true; } @@ -87,13 +86,8 @@ class AlwaysDelayingDelegate : public ResourceQueueDelegate { } void StartDelayedRequests() { - if (!resource_queue_) - return; - - for (RequestList::iterator i = delayed_requests_.begin(); - i != delayed_requests_.end(); ++i) { - resource_queue_->StartDelayedRequest(this, *i); - } + if (resource_queue_) + resource_queue_->StartDelayedRequests(this); } private: @@ -101,8 +95,6 @@ class AlwaysDelayingDelegate : public ResourceQueueDelegate { ResourceQueue* resource_queue_; - RequestList delayed_requests_; - DISALLOW_COPY_AND_ASSIGN(AlwaysDelayingDelegate); }; @@ -217,6 +209,32 @@ TEST_F(ResourceQueueTest, TwoDelegates) { queue.Shutdown(); } +TEST_F(ResourceQueueTest, TwoDelayingDelegates) { + ResourceQueue queue; + + AlwaysDelayingDelegate always_delaying_delegate1; + AlwaysDelayingDelegate always_delaying_delegate2; + InitializeQueue( + &queue, &always_delaying_delegate1, &always_delaying_delegate2); + + net::URLRequest request(GURL(kTestUrl), this); + scoped_ptr<ResourceDispatcherHostRequestInfo> request_info(GetRequestInfo(0)); + EXPECT_EQ(0, response_started_count_); + queue.AddRequest(&request, *request_info.get()); + MessageLoop::current()->RunAllPending(); + EXPECT_EQ(0, response_started_count_); + + always_delaying_delegate1.StartDelayedRequests(); + MessageLoop::current()->RunAllPending(); + EXPECT_EQ(0, response_started_count_); + + always_delaying_delegate2.StartDelayedRequests(); + MessageLoop::current()->RunAllPending(); + EXPECT_EQ(1, response_started_count_); + + queue.Shutdown(); +} + TEST_F(ResourceQueueTest, RemoveRequest) { ResourceQueue queue; diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h index a66c012..6b23f54 100644 --- a/net/url_request/url_request.h +++ b/net/url_request/url_request.h @@ -38,6 +38,7 @@ class ResourceDispatcherHostTest; class TestAutomationProvider; class URLRequestAutomationJob; class UserScriptListenerTest; +class NetworkDelayListenerTest; // Temporary layering violation to allow existing users of a deprecated // interface. @@ -175,6 +176,7 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe) { friend class ::ResourceDispatcherHostTest; friend class ::TestAutomationProvider; friend class ::UserScriptListenerTest; + friend class ::NetworkDelayListenerTest; friend class ::URLRequestAutomationJob; friend class TestInterceptor; friend class URLRequestFilter; |