diff options
author | avi@chromium.org <avi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-09-05 04:21:13 +0000 |
---|---|---|
committer | avi@chromium.org <avi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-09-05 04:21:13 +0000 |
commit | b0f724cb9ff5d2bfb59deb8f6deb14815dd93981 (patch) | |
tree | 0d138a229353b3868f22709d7560dc8e6ccbcd3c | |
parent | 94a519a3d02444882008ce712f2e19d257c76c49 (diff) | |
download | chromium_src-b0f724cb9ff5d2bfb59deb8f6deb14815dd93981.zip chromium_src-b0f724cb9ff5d2bfb59deb8f6deb14815dd93981.tar.gz chromium_src-b0f724cb9ff5d2bfb59deb8f6deb14815dd93981.tar.bz2 |
Create WebContentsObserver callbacks for notifications, remove notifications from SSLManager.
BUG=82582,170921
TEST=no functional change
NOTRY=true
Review URL: https://chromiumcodereview.appspot.com/23947003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@221342 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | content/browser/loader/resource_dispatcher_host_impl.cc | 36 | ||||
-rw-r--r-- | content/browser/renderer_host/render_view_host_delegate.h | 16 | ||||
-rw-r--r-- | content/browser/ssl/ssl_manager.cc | 72 | ||||
-rw-r--r-- | content/browser/ssl/ssl_manager.h | 32 | ||||
-rw-r--r-- | content/browser/web_contents/navigation_controller_impl.cc | 11 | ||||
-rw-r--r-- | content/browser/web_contents/navigation_controller_impl.h | 2 | ||||
-rw-r--r-- | content/browser/web_contents/web_contents_impl.cc | 38 | ||||
-rw-r--r-- | content/browser/web_contents/web_contents_impl.h | 4 | ||||
-rw-r--r-- | content/public/browser/notification_types.h | 3 | ||||
-rw-r--r-- | content/public/browser/web_contents_observer.h | 20 |
10 files changed, 125 insertions, 109 deletions
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc index 2a4ad9a..07d88b1 100644 --- a/content/browser/loader/resource_dispatcher_host_impl.cc +++ b/content/browser/loader/resource_dispatcher_host_impl.cc @@ -58,7 +58,6 @@ #include "content/public/browser/download_manager.h" #include "content/public/browser/download_url_parameters.h" #include "content/public/browser/global_request_id.h" -#include "content/public/browser/notification_service.h" #include "content/public/browser/resource_dispatcher_host_delegate.h" #include "content/public/browser/resource_request_details.h" #include "content/public/browser/resource_throttle.h" @@ -266,17 +265,28 @@ int GetCertID(net::URLRequest* request, int child_id) { return 0; } -template <class T> -void NotifyOnUI(int type, int render_process_id, int render_view_id, - scoped_ptr<T> detail) { +void NotifyRedirectOnUI(int render_process_id, + int render_view_id, + scoped_ptr<ResourceRedirectDetails> details) { RenderViewHostImpl* host = RenderViewHostImpl::FromID(render_process_id, render_view_id); - if (host) { - RenderViewHostDelegate* delegate = host->GetDelegate(); - NotificationService::current()->Notify( - type, Source<WebContents>(delegate->GetAsWebContents()), - Details<T>(detail.get())); - } + if (!host) + return; + + RenderViewHostDelegate* delegate = host->GetDelegate(); + delegate->DidGetRedirectForResourceRequest(*details.get()); +} + +void NotifyResponseOnUI(int render_process_id, + int render_view_id, + scoped_ptr<ResourceRequestDetails> details) { + RenderViewHostImpl* host = + RenderViewHostImpl::FromID(render_process_id, render_view_id); + if (!host) + return; + + RenderViewHostDelegate* delegate = host->GetDelegate(); + delegate->DidGetResourceResponseStart(*details.get()); } } // namespace @@ -681,8 +691,7 @@ void ResourceDispatcherHostImpl::DidReceiveRedirect(ResourceLoader* loader, BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, base::Bind( - &NotifyOnUI<ResourceRedirectDetails>, - static_cast<int>(NOTIFICATION_RESOURCE_RECEIVED_REDIRECT), + &NotifyRedirectOnUI, render_process_id, render_view_id, base::Passed(&detail))); } @@ -713,8 +722,7 @@ void ResourceDispatcherHostImpl::DidReceiveResponse(ResourceLoader* loader) { BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, base::Bind( - &NotifyOnUI<ResourceRequestDetails>, - static_cast<int>(NOTIFICATION_RESOURCE_RESPONSE_STARTED), + &NotifyResponseOnUI, render_process_id, render_view_id, base::Passed(&detail))); } diff --git a/content/browser/renderer_host/render_view_host_delegate.h b/content/browser/renderer_host/render_view_host_delegate.h index c19f8d2..aa3d704 100644 --- a/content/browser/renderer_host/render_view_host_delegate.h +++ b/content/browser/renderer_host/render_view_host_delegate.h @@ -50,6 +50,7 @@ class PageState; class RenderViewHost; class RenderViewHostDelegateView; class SessionStorageNamespace; +class SiteInstance; class WebContents; class WebContentsImpl; struct ContextMenuParams; @@ -58,7 +59,8 @@ struct GlobalRequestID; struct NativeWebKeyboardEvent; struct Referrer; struct RendererPreferences; -class SiteInstance; +struct ResourceRedirectDetails; +struct ResourceRequestDetails; // // RenderViewHostDelegate @@ -151,8 +153,8 @@ class CONTENT_EXPORT RenderViewHostDelegate { // The RenderView processed a redirect during a provisional load. // // TODO(creis): Remove this method and have the pre-rendering code listen to - // the ResourceDispatcherHost's RESOURCE_RECEIVED_REDIRECT notification - // instead. See http://crbug.com/78512. + // WebContentsObserver::DidGetRedirectForResourceRequest instead. + // See http://crbug.com/78512. virtual void DidRedirectProvisionalLoad( RenderViewHost* render_view_host, int32 page_id, @@ -164,6 +166,14 @@ class CONTENT_EXPORT RenderViewHostDelegate { RenderViewHost* render_view_host, const ViewHostMsg_DidFailProvisionalLoadWithError_Params& params) {} + // A response has been received for a resource request. + virtual void DidGetResourceResponseStart( + const ResourceRequestDetails& details) {} + + // A redirect was received while requesting a resource. + virtual void DidGetRedirectForResourceRequest( + const ResourceRedirectDetails& details) {} + // The RenderView was navigated to a different page. virtual void DidNavigate(RenderViewHost* render_view_host, const ViewHostMsg_FrameNavigate_Params& params) {} diff --git a/content/browser/ssl/ssl_manager.cc b/content/browser/ssl/ssl_manager.cc index c86dccd..42ee02f 100644 --- a/content/browser/ssl/ssl_manager.cc +++ b/content/browser/ssl/ssl_manager.cc @@ -21,8 +21,6 @@ #include "content/public/browser/browser_thread.h" #include "content/public/browser/load_from_memory_cache_details.h" #include "content/public/browser/navigation_details.h" -#include "content/public/browser/notification_service.h" -#include "content/public/browser/notification_source.h" #include "content/public/browser/resource_request_details.h" #include "content/public/common/ssl_status.h" #include "net/url_request/url_request.h" @@ -99,17 +97,6 @@ SSLManager::SSLManager(NavigationControllerImpl* controller) controller_(controller) { DCHECK(controller_); - // Subscribe to various notifications. - registrar_.Add( - this, NOTIFICATION_RESOURCE_RESPONSE_STARTED, - Source<WebContents>(controller_->web_contents())); - registrar_.Add( - this, NOTIFICATION_RESOURCE_RECEIVED_REDIRECT, - Source<WebContents>(controller_->web_contents())); - registrar_.Add( - this, NOTIFICATION_LOAD_FROM_MEMORY_CACHE, - Source<NavigationController>(controller_)); - SSLManagerSet* managers = static_cast<SSLManagerSet*>( controller_->GetBrowserContext()->GetUserData(kSSLManagerKeyName)); if (!managers) { @@ -125,22 +112,18 @@ SSLManager::~SSLManager() { managers->get().erase(this); } -void SSLManager::DidCommitProvisionalLoad( - const NotificationDetails& in_details) { - LoadCommittedDetails* details = - Details<LoadCommittedDetails>(in_details).ptr(); - +void SSLManager::DidCommitProvisionalLoad(const LoadCommittedDetails& details) { NavigationEntryImpl* entry = NavigationEntryImpl::FromNavigationEntry(controller_->GetActiveEntry()); - if (details->is_main_frame) { + if (details.is_main_frame) { if (entry) { // Decode the security details. int ssl_cert_id; net::CertStatus ssl_cert_status; int ssl_security_bits; int ssl_connection_status; - DeserializeSecurityInfo(details->serialized_security_info, + DeserializeSecurityInfo(details.serialized_security_info, &ssl_cert_id, &ssl_cert_status, &ssl_security_bits, @@ -171,52 +154,32 @@ void SSLManager::DidRunInsecureContent(const std::string& security_origin) { UpdateEntry(navigation_entry); } -void SSLManager::Observe(int type, - const NotificationSource& source, - const NotificationDetails& details) { - // Dispatch by type. - switch (type) { - case NOTIFICATION_RESOURCE_RESPONSE_STARTED: - DidStartResourceResponse( - Details<ResourceRequestDetails>(details).ptr()); - break; - case NOTIFICATION_RESOURCE_RECEIVED_REDIRECT: - DidReceiveResourceRedirect( - Details<ResourceRedirectDetails>(details).ptr()); - break; - case NOTIFICATION_LOAD_FROM_MEMORY_CACHE: - DidLoadFromMemoryCache( - Details<LoadFromMemoryCacheDetails>(details).ptr()); - break; - default: - NOTREACHED() << "The SSLManager received an unexpected notification."; - } -} - -void SSLManager::DidLoadFromMemoryCache(LoadFromMemoryCacheDetails* details) { +void SSLManager::DidLoadFromMemoryCache( + const LoadFromMemoryCacheDetails& details) { // Simulate loading this resource through the usual path. // Note that we specify SUB_RESOURCE as the resource type as WebCore only // caches sub-resources. // This resource must have been loaded with no filtering because filtered // resouces aren't cachable. scoped_refptr<SSLRequestInfo> info(new SSLRequestInfo( - details->url, + details.url, ResourceType::SUB_RESOURCE, - details->pid, - details->cert_id, - details->cert_status)); + details.pid, + details.cert_id, + details.cert_status)); // Simulate loading this resource through the usual path. policy()->OnRequestStarted(info.get()); } -void SSLManager::DidStartResourceResponse(ResourceRequestDetails* details) { +void SSLManager::DidStartResourceResponse( + const ResourceRequestDetails& details) { scoped_refptr<SSLRequestInfo> info(new SSLRequestInfo( - details->url, - details->resource_type, - details->origin_child_id, - details->ssl_cert_id, - details->ssl_cert_status)); + details.url, + details.resource_type, + details.origin_child_id, + details.ssl_cert_id, + details.ssl_cert_status)); // Notify our policy that we started a resource request. Ideally, the // policy should have the ability to cancel the request, but we can't do @@ -224,7 +187,8 @@ void SSLManager::DidStartResourceResponse(ResourceRequestDetails* details) { policy()->OnRequestStarted(info.get()); } -void SSLManager::DidReceiveResourceRedirect(ResourceRedirectDetails* details) { +void SSLManager::DidReceiveResourceRedirect( + const ResourceRedirectDetails& details) { // TODO(abarth): Make sure our redirect behavior is correct. If we ever see a // non-HTTPS resource in the redirect chain, we want to trigger // insecure content, even if the redirect chain goes back to diff --git a/content/browser/ssl/ssl_manager.h b/content/browser/ssl/ssl_manager.h index 6c80edd..45100a7 100644 --- a/content/browser/ssl/ssl_manager.h +++ b/content/browser/ssl/ssl_manager.h @@ -14,8 +14,6 @@ #include "content/browser/ssl/ssl_policy_backend.h" #include "content/common/content_export.h" #include "content/public/browser/global_request_id.h" -#include "content/public/browser/notification_observer.h" -#include "content/public/browser/notification_registrar.h" #include "net/base/net_errors.h" #include "net/cert/cert_status_flags.h" #include "url/gurl.h" @@ -29,6 +27,7 @@ class BrowserContext; class NavigationEntryImpl; class NavigationControllerImpl; class SSLPolicy; +struct LoadCommittedDetails; struct LoadFromMemoryCacheDetails; struct ResourceRedirectDetails; struct ResourceRequestDetails; @@ -41,7 +40,7 @@ struct ResourceRequestDetails; // The security state (secure/insecure) is stored in the navigation entry. // Along with it are stored any SSL error code and the associated cert. -class SSLManager : public NotificationObserver { +class SSLManager { public: // Entry point for SSLCertificateErrors. This function begins the process // of resolving a certificate error during an SSL connection. SSLManager @@ -74,32 +73,16 @@ class SSLManager : public NotificationObserver { // NavigationController is guaranteed to outlive the SSLManager. NavigationControllerImpl* controller() { return controller_; } - // This entry point is called directly (instead of via the notification - // service) because we need more precise control of the order in which folks - // are notified of this event. - void DidCommitProvisionalLoad(const NotificationDetails& details); + void DidCommitProvisionalLoad(const LoadCommittedDetails& details); + void DidLoadFromMemoryCache(const LoadFromMemoryCacheDetails& details); + void DidStartResourceResponse(const ResourceRequestDetails& details); + void DidReceiveResourceRedirect(const ResourceRedirectDetails& details); // Insecure content entry point. void DidDisplayInsecureContent(); void DidRunInsecureContent(const std::string& security_origin); - // Entry point for navigation. This function begins the process of updating - // the security UI when the main frame navigates to a new URL. - // - // Called on the UI thread. - virtual void Observe(int type, - const NotificationSource& source, - const NotificationDetails& details) OVERRIDE; - private: - // Entry points for notifications to which we subscribe. Note that - // DidCommitProvisionalLoad uses the abstract NotificationDetails type since - // the type we need is in NavigationController which would create a circular - // header file dependency. - void DidLoadFromMemoryCache(LoadFromMemoryCacheDetails* details); - void DidStartResourceResponse(ResourceRequestDetails* details); - void DidReceiveResourceRedirect(ResourceRedirectDetails* details); - // Update the NavigationEntry with our current state. void UpdateEntry(NavigationEntryImpl* entry); @@ -113,9 +96,6 @@ class SSLManager : public NotificationObserver { // for the security UI of this tab. NavigationControllerImpl* controller_; - // Handles registering notifications with the NotificationService. - NotificationRegistrar registrar_; - DISALLOW_COPY_AND_ASSIGN(SSLManager); }; diff --git a/content/browser/web_contents/navigation_controller_impl.cc b/content/browser/web_contents/navigation_controller_impl.cc index c4a0c24..36f2b00 100644 --- a/content/browser/web_contents/navigation_controller_impl.cc +++ b/content/browser/web_contents/navigation_controller_impl.cc @@ -1558,21 +1558,18 @@ void NavigationControllerImpl::NavigateToPendingEntry(ReloadType reload_type) { void NavigationControllerImpl::NotifyNavigationEntryCommitted( LoadCommittedDetails* details) { details->entry = GetActiveEntry(); - NotificationDetails notification_details = - Details<LoadCommittedDetails>(details); // We need to notify the ssl_manager_ before the web_contents_ so the // location bar will have up-to-date information about the security style // when it wants to draw. See http://crbug.com/11157 - ssl_manager_.DidCommitProvisionalLoad(notification_details); + ssl_manager_.DidCommitProvisionalLoad(*details); - // TODO(pkasting): http://b/1113079 Probably these explicit notification paths - // should be removed, and interested parties should just listen for the - // notification below instead. web_contents_->NotifyNavigationStateChanged(kInvalidateAll); - web_contents_->NotifyNavigationEntryCommitted(*details); + // TODO(avi): Remove. http://crbug.com/170921 + NotificationDetails notification_details = + Details<LoadCommittedDetails>(details); NotificationService::current()->Notify( NOTIFICATION_NAV_ENTRY_COMMITTED, Source<NavigationController>(this), diff --git a/content/browser/web_contents/navigation_controller_impl.h b/content/browser/web_contents/navigation_controller_impl.h index bef3be2..0bc5208 100644 --- a/content/browser/web_contents/navigation_controller_impl.h +++ b/content/browser/web_contents/navigation_controller_impl.h @@ -363,7 +363,7 @@ class CONTENT_EXPORT NavigationControllerImpl // of the restored entries to update its max page ID. int32 max_restored_page_id_; - // Manages the SSL security UI + // Manages the SSL security UI. SSLManager ssl_manager_; // Whether we need to be reloaded when made active. diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index f0abbf2..db344d9 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -2179,8 +2179,8 @@ void WebContentsImpl::DidRedirectProvisionalLoad( const GURL& source_url, const GURL& target_url) { // TODO(creis): Remove this method and have the pre-rendering code listen to - // the ResourceDispatcherHost's RESOURCE_RECEIVED_REDIRECT notification - // instead. See http://crbug.com/78512. + // WebContentsObserver::DidGetRedirectForResourceRequest instead. + // See http://crbug.com/78512. GURL validated_source_url(source_url); GURL validated_target_url(target_url); RenderProcessHost* render_process_host = @@ -2279,6 +2279,12 @@ void WebContentsImpl::OnDidLoadResourceFromMemoryCache( url, GetRenderProcessHost()->GetID(), cert_id, cert_status, http_method, mime_type, resource_type); + controller_.ssl_manager()->DidLoadFromMemoryCache(details); + + FOR_EACH_OBSERVER(WebContentsObserver, observers_, + DidLoadResourceFromMemoryCache(details)); + + // TODO(avi): Remove. http://crbug.com/170921 NotificationService::current()->Notify( NOTIFICATION_LOAD_FROM_MEMORY_CACHE, Source<NavigationController>(&controller_), @@ -2939,6 +2945,34 @@ void WebContentsImpl::RenderViewDeleted(RenderViewHost* rvh) { FOR_EACH_OBSERVER(WebContentsObserver, observers_, RenderViewDeleted(rvh)); } +void WebContentsImpl::DidGetResourceResponseStart( + const ResourceRequestDetails& details) { + controller_.ssl_manager()->DidStartResourceResponse(details); + + FOR_EACH_OBSERVER(WebContentsObserver, observers_, + DidGetResourceResponseStart(details)); + + // TODO(avi): Remove. http://crbug.com/170921 + NotificationService::current()->Notify( + NOTIFICATION_RESOURCE_RESPONSE_STARTED, + Source<WebContents>(this), + Details<const ResourceRequestDetails>(&details)); +} + +void WebContentsImpl::DidGetRedirectForResourceRequest( + const ResourceRedirectDetails& details) { + controller_.ssl_manager()->DidReceiveResourceRedirect(details); + + FOR_EACH_OBSERVER(WebContentsObserver, observers_, + DidGetRedirectForResourceRequest(details)); + + // TODO(avi): Remove. http://crbug.com/170921 + NotificationService::current()->Notify( + NOTIFICATION_RESOURCE_RECEIVED_REDIRECT, + Source<WebContents>(this), + Details<const ResourceRedirectDetails>(&details)); +} + void WebContentsImpl::DidNavigate( RenderViewHost* rvh, const ViewHostMsg_FrameNavigate_Params& params) { diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h index 8fead69..6b9f1ba 100644 --- a/content/browser/web_contents/web_contents_impl.h +++ b/content/browser/web_contents/web_contents_impl.h @@ -347,6 +347,10 @@ class CONTENT_EXPORT WebContentsImpl RenderViewHost* render_view_host, const ViewHostMsg_DidFailProvisionalLoadWithError_Params& params) OVERRIDE; + virtual void DidGetResourceResponseStart( + const ResourceRequestDetails& details) OVERRIDE; + virtual void DidGetRedirectForResourceRequest( + const ResourceRedirectDetails& details) OVERRIDE; virtual void DidNavigate( RenderViewHost* render_view_host, const ViewHostMsg_FrameNavigate_Params& params) OVERRIDE; diff --git a/content/public/browser/notification_types.h b/content/public/browser/notification_types.h index db8654c..01eec42 100644 --- a/content/public/browser/notification_types.h +++ b/content/public/browser/notification_types.h @@ -89,17 +89,20 @@ enum NotificationType { // Source<NavigationController> corresponding to the tab in which the load // occurred. Details in the form of a LoadFromMemoryCacheDetails object // are provided. + // DEPRECATED: Use WebContentsObserver::DidLoadResourceFromMemoryCache() NOTIFICATION_LOAD_FROM_MEMORY_CACHE, // A response has been received for a resource request. The source will be // a Source<WebContents> corresponding to the tab in which the request was // issued. Details in the form of a ResourceRequestDetails object are // provided. + // DEPRECATED: Use WebContentsObserver::DidGetResourceResponseStart() NOTIFICATION_RESOURCE_RESPONSE_STARTED, // A redirect was received while requesting a resource. The source will be // a Source<WebContents> corresponding to the tab in which the request was // issued. Details in the form of a ResourceRedirectDetails are provided. + // DEPRECATED: Use WebContentsObserver::DidGetRedirectForResourceRequest() NOTIFICATION_RESOURCE_RECEIVED_REDIRECT, // WebContents --------------------------------------------------------------- diff --git a/content/public/browser/web_contents_observer.h b/content/public/browser/web_contents_observer.h index d46f315..0057e38 100644 --- a/content/public/browser/web_contents_observer.h +++ b/content/public/browser/web_contents_observer.h @@ -22,7 +22,10 @@ class WebContentsImpl; struct FaviconURL; struct FrameNavigateParams; struct LoadCommittedDetails; +struct LoadFromMemoryCacheDetails; struct Referrer; +struct ResourceRedirectDetails; +struct ResourceRequestDetails; // An observer API implemented by classes which are interested in various page // load events from WebContents. They also get a chance to filter IPC messages. @@ -97,8 +100,7 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener, // This method is invoked right after the DidStartProvisionalLoadForFrame if // the provisional load affects the main frame, or if the provisional load // was redirected. The latter use case is DEPRECATED. You should listen to - // the ResourceDispatcherHost's RESOURCE_RECEIVED_REDIRECT notification - // instead. + // WebContentsObserver::DidGetRedirectForResourceRequest instead. virtual void ProvisionalChangeToMainFrameUrl( const GURL& url, RenderViewHost* render_view_host) {} @@ -166,6 +168,20 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener, const string16& error_description, RenderViewHost* render_view_host) {} + // This method is invoked when content was loaded from an in-memory cache. + virtual void DidLoadResourceFromMemoryCache( + const LoadFromMemoryCacheDetails& details) {} + + // This method is invoked when a response has been received for a resource + // request. + virtual void DidGetResourceResponseStart( + const ResourceRequestDetails& details) {} + + // This method is invoked when a redirect was received while requesting a + // resource. + virtual void DidGetRedirectForResourceRequest( + const ResourceRedirectDetails& details) {} + // This method is invoked when a new non-pending navigation entry is created. // This corresponds to one NavigationController entry being created // (in the case of new navigations) or renavigated to (for back/forward |