Skip to content

Commit

Permalink
Refactored blocked_loaders_map_ to key by render frame route id
Browse files Browse the repository at this point in the history
This change is a necessary step to completely refactor out render view ids from the ResourceDispatcherHostImpl.

This means that the interface to block, resume, or cancel requests now require passing in the frame routing id and the child id.

The ResourceScheduler, SaveFile, and Downloads still use the RVID.
BUG=472869
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

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

Cr-Commit-Position: refs/heads/master@{#373380}
  • Loading branch information
csharrison authored and Commit bot committed Feb 3, 2016
1 parent 34cfec9 commit a2280cd
Show file tree
Hide file tree
Showing 24 changed files with 438 additions and 204 deletions.
1 change: 1 addition & 0 deletions content/browser/browser_plugin/browser_plugin_guest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,7 @@ void BrowserPluginGuest::OnWillAttachComplete(
static_cast<RenderViewHostImpl*>(GetWebContents()->GetRenderViewHost())
->GetWidget()
->Init();
GetWebContents()->GetMainFrame()->Init();
WebContentsViewGuest* web_contents_view =
static_cast<WebContentsViewGuest*>(GetWebContents()->GetView());
if (!web_contents()->GetRenderViewHost()->GetWidget()->GetView()) {
Expand Down
49 changes: 15 additions & 34 deletions content/browser/frame_host/interstitial_page_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,28 +53,6 @@ using blink::WebDragOperation;
using blink::WebDragOperationsMask;

namespace content {
namespace {

void ResourceRequestHelper(ResourceDispatcherHostImpl* rdh,
int process_id,
int render_view_host_id,
ResourceRequestAction action) {
switch (action) {
case BLOCK:
rdh->BlockRequestsForRoute(process_id, render_view_host_id);
break;
case RESUME:
rdh->ResumeBlockedRequestsForRoute(process_id, render_view_host_id);
break;
case CANCEL:
rdh->CancelBlockedRequestsForRoute(process_id, render_view_host_id);
break;
default:
NOTREACHED();
}
}

} // namespace

class InterstitialPageImpl::InterstitialPageRVHDelegateView
: public RenderViewHostDelegateView {
Expand Down Expand Up @@ -855,22 +833,25 @@ void InterstitialPageImpl::TakeActionOnResourceDispatcher(
// 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 a ResourceDispatcherHostImpl, as when unit-
// tests we don't have one.
RenderViewHostImpl* rvh = RenderViewHostImpl::FromID(original_child_id_,
original_rvh_id_);
if (!rvh || !ResourceDispatcherHostImpl::Get())
if (!rvh)
return;

BrowserThread::PostTask(
BrowserThread::IO,
FROM_HERE,
base::Bind(
&ResourceRequestHelper,
ResourceDispatcherHostImpl::Get(),
original_child_id_,
original_rvh_id_,
action));
RenderFrameHostImpl* rfh =
static_cast<RenderFrameHostImpl*>(rvh->GetMainFrame());
switch (action) {
case BLOCK:
ResourceDispatcherHost::BlockRequestsForFrameFromUI(rfh);
break;
case RESUME:
ResourceDispatcherHost::ResumeBlockedRequestsForFrameFromUI(rfh);
break;
default:
DCHECK_EQ(action, CANCEL);
ResourceDispatcherHostImpl::CancelBlockedRequestsForFrameFromUI(rfh);
break;
}
}

void InterstitialPageImpl::OnDomOperationResponse(
Expand Down
9 changes: 6 additions & 3 deletions content/browser/frame_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "content/browser/frame_host/render_frame_proxy_host.h"
#include "content/browser/frame_host/render_widget_host_view_child_frame.h"
#include "content/browser/geolocation/geolocation_service_context.h"
#include "content/browser/loader/resource_dispatcher_host_impl.h"
#include "content/browser/permissions/permission_service_context.h"
#include "content/browser/permissions/permission_service_impl.h"
#include "content/browser/presentation/presentation_service_impl.h"
Expand Down Expand Up @@ -779,9 +780,7 @@ void RenderFrameHostImpl::SetRenderFrameCreated(bool created) {
}

void RenderFrameHostImpl::Init() {
// TODO(csharrison): Call GetProcess()->ResumeRequestsForFrame(routing_id_)
// once ResourceDispatcherHostImpl is keyed on render frame routing ids
// instead of render view routing ids.
ResourceDispatcherHost::ResumeBlockedRequestsForFrameFromUI(this);
}

void RenderFrameHostImpl::OnAddMessageToConsole(
Expand Down Expand Up @@ -1077,6 +1076,10 @@ RenderWidgetHostView* RenderFrameHostImpl::GetView() {
return nullptr;
}

GlobalFrameRoutingId RenderFrameHostImpl::GetGlobalFrameRoutingId() {
return GlobalFrameRoutingId(GetProcess()->GetID(), GetRoutingID());
}

int RenderFrameHostImpl::GetEnabledBindings() {
return render_view_host_->GetEnabledBindings();
}
Expand Down
2 changes: 2 additions & 0 deletions content/browser/frame_host/render_frame_host_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "build/build_config.h"
#include "content/browser/accessibility/browser_accessibility_manager.h"
#include "content/browser/bad_message.h"
#include "content/browser/loader/global_routing_id.h"
#include "content/browser/site_instance_impl.h"
#include "content/browser/webui/web_ui_impl.h"
#include "content/common/accessibility_mode_enums.h"
Expand Down Expand Up @@ -252,6 +253,7 @@ class CONTENT_EXPORT RenderFrameHostImpl
// pointer to the RenderViewHost (which inherits RenderWidgetHost).
RenderWidgetHostImpl* GetRenderWidgetHost();

GlobalFrameRoutingId GetGlobalFrameRoutingId();

// This function is called when this is a swapped out RenderFrameHost that
// lives in the same process as the parent frame. The
Expand Down
17 changes: 13 additions & 4 deletions content/browser/loader/cross_site_resource_handler_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ class TestResourceDispatcherHostDelegate
CallbackRunningResourceThrottle(net::URLRequest* request,
TestResourceDispatcherHostDelegate* tracker,
const RequestDeferredHook& run_on_start)
: request_(request),
: resumed_(false),
request_(request),
tracker_(tracker),
run_on_start_(run_on_start),
weak_factory_(this) {}
Expand All @@ -116,9 +117,12 @@ class TestResourceDispatcherHostDelegate
~CallbackRunningResourceThrottle() override {
// If the request is deleted without being cancelled, its status will
// indicate it succeeded, so have to check if the request is still pending
// as well.
// as well. If the request never even started, the throttle will never
// resume it. Check this condition as well to allow for early
// cancellation.
tracker_->OnTrackedRequestDestroyed(!request_->is_pending() &&
request_->status().is_success());
request_->status().is_success() &&
resumed_);
}

// ResourceThrottle implementation:
Expand All @@ -127,7 +131,12 @@ class TestResourceDispatcherHostDelegate
}

private:
void Resume() { controller()->Resume(); }
void Resume() {
resumed_ = true;
controller()->Resume();
}

bool resumed_;
net::URLRequest* request_;
TestResourceDispatcherHostDelegate* tracker_;
RequestDeferredHook run_on_start_;
Expand Down
29 changes: 29 additions & 0 deletions content/browser/loader/global_routing_id.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#include <tuple>

#include "ipc/ipc_message.h"

namespace content {

// Uniquely identifies the route from which a net::URLRequest comes.
Expand Down Expand Up @@ -38,6 +40,33 @@ struct GlobalRoutingID {
}
};

// Same as GlobalRoutingID except the route_id must be a RenderFrameHost routing
// id.
struct GlobalFrameRoutingId {
GlobalFrameRoutingId() : child_id(0), frame_routing_id(MSG_ROUTING_NONE) {}

GlobalFrameRoutingId(int child_id, int frame_routing_id)
: child_id(child_id), frame_routing_id(frame_routing_id) {}

// The unique ID of the child process (different from OS's PID).
int child_id;

// The route ID (unique for each URLRequest source).
int frame_routing_id;

bool operator<(const GlobalFrameRoutingId& other) const {
return std::tie(child_id, frame_routing_id) <
std::tie(other.child_id, other.frame_routing_id);
}
bool operator==(const GlobalFrameRoutingId& other) const {
return child_id == other.child_id &&
frame_routing_id == other.frame_routing_id;
}
bool operator!=(const GlobalFrameRoutingId& other) const {
return !(*this == other);
}
};

} // namespace content

#endif // CONTENT_BROWSER_LOADER_GLOBAL_ROUTING_ID_H_
Loading

0 comments on commit a2280cd

Please sign in to comment.