Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1440)

Unified Diff: chrome/browser/interstitial_page.cc

Issue 16546: Blocking resource request for hidden page when interstitial showing (Closed) Base URL: svn://chrome-svn/chrome/trunk/src/
Patch Set: '' Created 11 years, 11 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
« no previous file with comments | « chrome/browser/interstitial_page.h ('k') | chrome/browser/renderer_host/resource_dispatcher_host.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/interstitial_page.cc
===================================================================
--- chrome/browser/interstitial_page.cc (revision 7403)
+++ chrome/browser/interstitial_page.cc (working copy)
@@ -17,6 +17,55 @@
#include "chrome/views/window_delegate.h"
#include "net/base/escape.h"
+enum ResourceRequestAction {
+ BLOCK,
+ RESUME,
+ CANCEL
+};
+
+namespace {
+
+class ResourceRequestTask : public Task {
+ public:
+ ResourceRequestTask(RenderViewHost* render_view_host,
+ ResourceRequestAction action)
+ : action_(action),
+ process_id_(render_view_host->process()->host_id()),
+ render_view_host_id_(render_view_host->routing_id()),
+ resource_dispatcher_host_(
+ g_browser_process->resource_dispatcher_host()) {
+ }
+
+ virtual void Run() {
+ switch (action_) {
+ case BLOCK:
+ resource_dispatcher_host_->BlockRequestsForRenderView(
+ process_id_, render_view_host_id_);
+ break;
+ case RESUME:
+ resource_dispatcher_host_->ResumeBlockedRequestsForRenderView(
+ process_id_, render_view_host_id_);
+ break;
+ case CANCEL:
+ resource_dispatcher_host_->CancelBlockedRequestsForRenderView(
+ process_id_, render_view_host_id_);
+ break;
+ default:
+ NOTREACHED();
+ }
+ }
+
+ private:
+ ResourceRequestAction action_;
+ int process_id_;
+ int render_view_host_id_;
+ ResourceDispatcherHost* resource_dispatcher_host_;
+
+ DISALLOW_COPY_AND_ASSIGN(ResourceRequestTask);
+};
+
+} // namespace
+
// static
InterstitialPage::InterstitialPageMap*
InterstitialPage::tab_to_interstitial_page_ = NULL;
@@ -30,7 +79,8 @@
enabled_(true),
new_navigation_(new_navigation),
render_view_host_(NULL),
- should_revert_tab_title_(false) {
+ should_revert_tab_title_(false),
+ ui_loop_(MessageLoop::current()) {
InitInterstitialPageMap();
// It would be inconsistent to create an interstitial with no new navigation
// (which is the case when the interstitial was triggered by a sub-resource on
@@ -51,6 +101,16 @@
if (tab_->interstitial_page())
tab_->interstitial_page()->DontProceed();
+ // Block the resource requests for the render view host while it is hidden.
+ TakeActionOnResourceDispatcher(BLOCK);
+ // We need to be notified when the RenderViewHost is destroyed so we can
+ // cancel the blocked requests. We cannot do that on
+ // NOTIFY_TAB_CONTENTS_DESTROYED as at that point the RenderViewHost has
+ // already been destroyed.
+ notification_registrar_.Add(
+ this, NOTIFY_RENDER_WIDGET_HOST_DESTROYED,
+ Source<RenderWidgetHost>(tab_->render_view_host()));
+
// Update the tab_to_interstitial_page_ map.
InterstitialPageMap::const_iterator iter =
tab_to_interstitial_page_->find(tab_);
@@ -101,26 +161,36 @@
void InterstitialPage::Observe(NotificationType type,
const NotificationSource& source,
const NotificationDetails& details) {
- if (type == NOTIFY_NAV_ENTRY_PENDING) {
- // We are navigating away from the interstitial. Make sure clicking on the
- // interstitial will have no effect.
- Disable();
- return;
+ switch (type) {
+ case NOTIFY_NAV_ENTRY_PENDING:
+ // We are navigating away from the interstitial. Make sure clicking on
+ // the interstitial will have no effect.
+ Disable();
+ break;
+ case NOTIFY_RENDER_WIDGET_HOST_DESTROYED:
+ // The RenderViewHost is being destroyed (as part of the tab being closed)
+ // make sure we clear the blocked requests.
+ DCHECK(Source<RenderViewHost>(source).ptr() == tab_->render_view_host());
+ TakeActionOnResourceDispatcher(CANCEL);
+ break;
+ case NOTIFY_TAB_CONTENTS_DESTROYED:
+ case NOTIFY_NAV_ENTRY_COMMITTED:
+ if (!action_taken_) {
+ // We are navigating away from the interstitial or closing a tab with an
+ // interstitial. Default to DontProceed(). We don't just call Hide as
+ // subclasses will almost certainly override DontProceed to do some work
+ // (ex: close pending connections).
+ DontProceed();
+ } else {
+ // User decided to proceed and either the navigation was committed or
+ // the tab was closed before that.
+ Hide();
+ // WARNING: we are now deleted!
+ }
+ break;
+ default:
+ NOTREACHED();
}
- DCHECK(type == NOTIFY_TAB_CONTENTS_DESTROYED ||
- type == NOTIFY_NAV_ENTRY_COMMITTED);
- if (!action_taken_) {
- // We are navigating away from the interstitial or closing a tab with an
- // interstitial. Default to DontProceed(). We don't just call Hide as
- // subclasses will almost certainly override DontProceed to do some work
- // (ex: close pending connections).
- DontProceed();
- } else {
- // User decided to proceed and either the navigation was committed or the
- // tab was closed before that.
- Hide();
- // WARNING: we are now deleted!
- }
}
RenderViewHost* InterstitialPage::CreateRenderViewHost() {
@@ -150,6 +220,15 @@
// Resumes the throbber.
tab_->SetIsLoading(true, NULL);
+ // If this is a new navigation, the old page is going away, so we cancel any
+ // blocked requests for it. If it is not a new navigation, then it means the
+ // interstitial was shown as a result of a resource loading in the page.
+ // Since the user wants to proceed, we'll let any blocked request go through.
+ if (new_navigation_)
+ TakeActionOnResourceDispatcher(CANCEL);
+ else
+ TakeActionOnResourceDispatcher(RESUME);
+
// No need to hide if we are a new navigation, we'll get hidden when the
// navigation is committed.
if (!new_navigation_) {
@@ -163,6 +242,16 @@
Disable();
action_taken_ = true;
+ // If this is a new navigation, we are returning to the original page, so we
+ // resume blocked requests for it. If it is not a new navigation, then it
+ // means the interstitial was shown as a result of a resource loading in the
+ // page and we won't return to the original page, so we cancel blocked
+ // requests in that case.
+ if (new_navigation_)
+ TakeActionOnResourceDispatcher(RESUME);
+ else
+ TakeActionOnResourceDispatcher(CANCEL);
+
if (new_navigation_) {
// Since no navigation happens we have to discard the transient entry
// explicitely. Note that by calling DiscardNonCommittedEntries() we also
@@ -236,6 +325,21 @@
enabled_ = false;
}
+void InterstitialPage::TakeActionOnResourceDispatcher(
+ ResourceRequestAction action) {
+ DCHECK(MessageLoop::current() == ui_loop_) <<
+ "TakeActionOnResourceDispatcher should be called on the main thread.";
+ // The tab might not have a render_view_host if it was closed (in which case,
+ // we have taken care of the blocked requests when processing
+ // NOTIFY_RENDER_WIDGET_HOST_DESTROYED.
+ // Also we need to test there is an IO thread, as when unit-tests we don't
+ // have one.
+ if (tab_->render_view_host() && g_browser_process->io_thread()) {
+ g_browser_process->io_thread()->message_loop()->PostTask(
+ FROM_HERE, new ResourceRequestTask(tab_->render_view_host(), action));
+ }
+}
+
// static
void InterstitialPage::InitInterstitialPageMap() {
if (!tab_to_interstitial_page_)
« no previous file with comments | « chrome/browser/interstitial_page.h ('k') | chrome/browser/renderer_host/resource_dispatcher_host.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698