diff options
55 files changed, 2012 insertions, 428 deletions
diff --git a/chrome/browser/extensions/api/messaging/extension_message_port.cc b/chrome/browser/extensions/api/messaging/extension_message_port.cc index 1e4f87f..a22568f9 100644 --- a/chrome/browser/extensions/api/messaging/extension_message_port.cc +++ b/chrome/browser/extensions/api/messaging/extension_message_port.cc @@ -4,31 +4,121 @@ #include "chrome/browser/extensions/api/messaging/extension_message_port.h" +#include "base/scoped_observer.h" #include "chrome/browser/profiles/profile.h" +#include "content/public/browser/navigation_details.h" +#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_process_host.h" +#include "content/public/browser/web_contents.h" +#include "content/public/browser/web_contents_observer.h" #include "extensions/browser/extension_host.h" #include "extensions/browser/process_manager.h" +#include "extensions/browser/process_manager_observer.h" #include "extensions/common/extension_messages.h" #include "extensions/common/manifest_handlers/background_info.h" namespace extensions { -ExtensionMessagePort::ExtensionMessagePort(content::RenderProcessHost* process, - int routing_id, - const std::string& extension_id) - : process_(process), - routing_id_(routing_id), - extension_id_(extension_id), - background_host_ptr_(NULL) { +const char kReceivingEndDoesntExistError[] = + "Could not establish connection. Receiving end does not exist."; + +// Helper class to detect when frames are destroyed. +class ExtensionMessagePort::FrameTracker : public content::WebContentsObserver, + public ProcessManagerObserver { + public: + explicit FrameTracker(ExtensionMessagePort* port) + : pm_observer_(this), port_(port) {} + ~FrameTracker() override {} + + void TrackExtensionProcessFrames() { + pm_observer_.Add(ProcessManager::Get(port_->browser_context_)); + } + + void TrackTabFrames(content::WebContents* tab) { + Observe(tab); + } + + private: + // content::WebContentsObserver overrides: + void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) + override { + port_->UnregisterFrame(render_frame_host); + } + + void DidNavigateAnyFrame(content::RenderFrameHost* render_frame_host, + const content::LoadCommittedDetails& details, + const content::FrameNavigateParams&) override { + if (!details.is_in_page) + port_->UnregisterFrame(render_frame_host); + } + + // extensions::ProcessManagerObserver overrides: + void OnExtensionFrameUnregistered( + const std::string& extension_id, + content::RenderFrameHost* render_frame_host) override { + if (extension_id == port_->extension_id_) + port_->UnregisterFrame(render_frame_host); + } + + ScopedObserver<ProcessManager, ProcessManagerObserver> pm_observer_; + ExtensionMessagePort* port_; // Owns this FrameTracker. + + DISALLOW_COPY_AND_ASSIGN(FrameTracker); +}; + +ExtensionMessagePort::ExtensionMessagePort( + base::WeakPtr<MessageService> message_service, + int port_id, + const std::string& extension_id, + content::RenderProcessHost* extension_process) + : weak_message_service_(message_service), + port_id_(port_id), + extension_id_(extension_id), + browser_context_(extension_process->GetBrowserContext()), + extension_process_(extension_process), + frames_(ProcessManager::Get(browser_context_)-> + GetRenderFrameHostsForExtension(extension_id)), + did_create_port_(false), + background_host_ptr_(nullptr), + frame_tracker_(new FrameTracker(this)) { + frame_tracker_->TrackExtensionProcessFrames(); +} + +ExtensionMessagePort::ExtensionMessagePort( + base::WeakPtr<MessageService> message_service, + int port_id, + const std::string& extension_id, + content::RenderFrameHost* rfh, + bool include_child_frames) + : weak_message_service_(message_service), + port_id_(port_id), + extension_id_(extension_id), + browser_context_(rfh->GetProcess()->GetBrowserContext()), + extension_process_(nullptr), + did_create_port_(false), + background_host_ptr_(nullptr), + frame_tracker_(new FrameTracker(this)) { + content::WebContents* tab = content::WebContents::FromRenderFrameHost(rfh); + DCHECK(tab); + frame_tracker_->TrackTabFrames(tab); + if (include_child_frames) { + tab->ForEachFrame(base::Bind(&ExtensionMessagePort::RegisterFrame, + base::Unretained(this))); + } else { + RegisterFrame(rfh); + } +} + +ExtensionMessagePort::~ExtensionMessagePort() {} + +bool ExtensionMessagePort::IsValidPort() { + return !frames_.empty(); } void ExtensionMessagePort::DispatchOnConnect( - int dest_port_id, const std::string& channel_name, scoped_ptr<base::DictionaryValue> source_tab, int source_frame_id, - int target_tab_id, - int target_frame_id, int guest_process_id, int guest_render_frame_routing_id, const std::string& source_extension_id, @@ -44,32 +134,26 @@ void ExtensionMessagePort::DispatchOnConnect( info.target_id = target_extension_id; info.source_id = source_extension_id; info.source_url = source_url; - info.target_tab_id = target_tab_id; - info.target_frame_id = target_frame_id; info.guest_process_id = guest_process_id; info.guest_render_frame_routing_id = guest_render_frame_routing_id; - process_->Send(new ExtensionMsg_DispatchOnConnect( - routing_id_, dest_port_id, channel_name, source, info, tls_channel_id)); + SendToPort(make_scoped_ptr(new ExtensionMsg_DispatchOnConnect( + MSG_ROUTING_NONE, port_id_, channel_name, source, info, tls_channel_id))); } void ExtensionMessagePort::DispatchOnDisconnect( - int source_port_id, const std::string& error_message) { - process_->Send(new ExtensionMsg_DispatchOnDisconnect( - routing_id_, source_port_id, error_message)); + SendToPort(make_scoped_ptr(new ExtensionMsg_DispatchOnDisconnect( + MSG_ROUTING_NONE, port_id_, error_message))); } -void ExtensionMessagePort::DispatchOnMessage(const Message& message, - int target_port_id) { - process_->Send(new ExtensionMsg_DeliverMessage( - routing_id_, target_port_id, message)); +void ExtensionMessagePort::DispatchOnMessage(const Message& message) { + SendToPort(make_scoped_ptr(new ExtensionMsg_DeliverMessage( + MSG_ROUTING_NONE, port_id_, message))); } void ExtensionMessagePort::IncrementLazyKeepaliveCount() { - Profile* profile = - Profile::FromBrowserContext(process_->GetBrowserContext()); - extensions::ProcessManager* pm = ProcessManager::Get(profile); + ProcessManager* pm = ProcessManager::Get(browser_context_); ExtensionHost* host = pm->GetBackgroundHostForExtension(extension_id_); if (host && BackgroundInfo::HasLazyBackgroundPage(host->extension())) pm->IncrementLazyKeepaliveCount(host->extension()); @@ -80,16 +164,66 @@ void ExtensionMessagePort::IncrementLazyKeepaliveCount() { } void ExtensionMessagePort::DecrementLazyKeepaliveCount() { - Profile* profile = - Profile::FromBrowserContext(process_->GetBrowserContext()); - extensions::ProcessManager* pm = ProcessManager::Get(profile); + ProcessManager* pm = ProcessManager::Get(browser_context_); ExtensionHost* host = pm->GetBackgroundHostForExtension(extension_id_); if (host && host == background_host_ptr_) pm->DecrementLazyKeepaliveCount(host->extension()); } -content::RenderProcessHost* ExtensionMessagePort::GetRenderProcessHost() { - return process_; +void ExtensionMessagePort::OpenPort(int process_id, int routing_id) { + DCHECK(routing_id != MSG_ROUTING_NONE || extension_process_); + + did_create_port_ = true; +} + +void ExtensionMessagePort::ClosePort(int process_id, int routing_id) { + if (routing_id == MSG_ROUTING_NONE) { + // The only non-frame-specific message is the response to an unhandled + // onConnect event in the extension process. + DCHECK(extension_process_); + frames_.clear(); + CloseChannel(); + return; + } + + content::RenderFrameHost* rfh = + content::RenderFrameHost::FromID(process_id, routing_id); + if (rfh) + UnregisterFrame(rfh); +} + +void ExtensionMessagePort::CloseChannel() { + std::string error_message = did_create_port_ ? std::string() : + kReceivingEndDoesntExistError; + if (weak_message_service_) + weak_message_service_->CloseChannel(port_id_, error_message); +} + +void ExtensionMessagePort::RegisterFrame(content::RenderFrameHost* rfh) { + frames_.insert(rfh); +} + +void ExtensionMessagePort::UnregisterFrame(content::RenderFrameHost* rfh) { + if (frames_.erase(rfh) != 0 && frames_.empty()) + CloseChannel(); +} + +void ExtensionMessagePort::SendToPort(scoped_ptr<IPC::Message> msg) { + DCHECK_GT(frames_.size(), 0UL); + if (extension_process_) { + // All extension frames reside in the same process, so we can just send a + // single IPC message to the extension process as an optimization. + // The frame tracking is then only used to make sure that the port gets + // closed when all frames have closed / reloaded. + msg->set_routing_id(MSG_ROUTING_CONTROL); + extension_process_->Send(msg.release()); + return; + } + for (content::RenderFrameHost* rfh : frames_) { + IPC::Message* msg_copy = new IPC::Message(*msg.get()); + msg_copy->set_routing_id(rfh->GetRoutingID()); + rfh->Send(msg_copy); + } } } // namespace extensions diff --git a/chrome/browser/extensions/api/messaging/extension_message_port.h b/chrome/browser/extensions/api/messaging/extension_message_port.h index 79b8003..f950f64 100644 --- a/chrome/browser/extensions/api/messaging/extension_message_port.h +++ b/chrome/browser/extensions/api/messaging/extension_message_port.h @@ -5,46 +5,101 @@ #ifndef CHROME_BROWSER_EXTENSIONS_API_MESSAGING_EXTENSION_MESSAGE_PORT_H_ #define CHROME_BROWSER_EXTENSIONS_API_MESSAGING_EXTENSION_MESSAGE_PORT_H_ +#include "base/macros.h" #include "chrome/browser/extensions/api/messaging/message_service.h" class GURL; namespace content { +class BrowserContext; +class RenderFrameHost; class RenderProcessHost; } // namespace content +namespace IPC { +class Message; +} // namespace IPC + namespace extensions { // A port that manages communication with an extension. +// The port's lifetime will end when either all receivers close the port, or +// when the opener / receiver explicitly closes the channel. class ExtensionMessagePort : public MessageService::MessagePort { public: - ExtensionMessagePort(content::RenderProcessHost* process, - int routing_id, - const std::string& extension_id); - void DispatchOnConnect(int dest_port_id, - const std::string& channel_name, + // Create a port that is tied to frame(s) in a single tab. + ExtensionMessagePort(base::WeakPtr<MessageService> message_service, + int port_id, + const std::string& extension_id, + content::RenderFrameHost* rfh, + bool include_child_frames); + // Create a port that is tied to all frames of an extension, possibly spanning + // multiple tabs, including the invisible background page, popups, etc. + ExtensionMessagePort(base::WeakPtr<MessageService> message_service, + int port_id, + const std::string& extension_id, + content::RenderProcessHost* extension_process); + ~ExtensionMessagePort() override; + + bool IsValidPort() override; + + // MessageService::MessagePort: + void DispatchOnConnect(const std::string& channel_name, scoped_ptr<base::DictionaryValue> source_tab, int source_frame_id, - int target_tab_id, - int target_frame_id, int guest_process_id, int guest_render_frame_routing_id, const std::string& source_extension_id, const std::string& target_extension_id, const GURL& source_url, const std::string& tls_channel_id) override; - void DispatchOnDisconnect(int source_port_id, - const std::string& error_message) override; - void DispatchOnMessage(const Message& message, int target_port_id) override; + void DispatchOnDisconnect(const std::string& error_message) override; + void DispatchOnMessage(const Message& message) override; void IncrementLazyKeepaliveCount() override; void DecrementLazyKeepaliveCount() override; - content::RenderProcessHost* GetRenderProcessHost() override; + void OpenPort(int process_id, int routing_id) override; + void ClosePort(int process_id, int routing_id) override; private: - content::RenderProcessHost* process_; - int routing_id_; + class FrameTracker; + + // Registers a frame as a receiver / sender. + void RegisterFrame(content::RenderFrameHost* rfh); + + // Unregisters a frame as a receiver / sender. When there are no registered + // frames any more, the port closes via CloseChannel(). + void UnregisterFrame(content::RenderFrameHost* rfh); + + // Immediately close the port and its associated channel. + void CloseChannel(); + + // Send a IPC message to the renderer for all registered frames. + void SendToPort(scoped_ptr<IPC::Message> msg); + + base::WeakPtr<MessageService> weak_message_service_; + + int port_id_; std::string extension_id_; - void* background_host_ptr_; // used in IncrementLazyKeepaliveCount + content::BrowserContext* browser_context_; + // Only for receivers in an extension process. + content::RenderProcessHost* extension_process_; + + // When the port is used as a sender, this set contains only one element. + // If used as a receiver, it may contain any number of frames. + // This set is populated before the first message is sent to the destination, + // and shrinks over time when the port is rejected by the recipient frame, or + // when the frame is removed or unloaded. + std::set<content::RenderFrameHost*> frames_; + + // Whether the renderer acknowledged creation of the port. This is used to + // distinguish abnormal port closure (e.g. no receivers) from explicit port + // closure (e.g. by the port.disconnect() JavaScript method in the renderer). + bool did_create_port_; + + ExtensionHost* background_host_ptr_; // used in IncrementLazyKeepaliveCount + scoped_ptr<FrameTracker> frame_tracker_; + + DISALLOW_COPY_AND_ASSIGN(ExtensionMessagePort); }; } // namespace extensions diff --git a/chrome/browser/extensions/api/messaging/message_service.cc b/chrome/browser/extensions/api/messaging/message_service.cc index 743ba0c..e936b7d 100644 --- a/chrome/browser/extensions/api/messaging/message_service.cc +++ b/chrome/browser/extensions/api/messaging/message_service.cc @@ -18,7 +18,6 @@ #include "base/prefs/pref_service.h" #include "base/stl_util.h" #include "build/build_config.h" -#include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/extensions/api/messaging/extension_message_port.h" #include "chrome/browser/extensions/api/messaging/incognito_connectability.h" #include "chrome/browser/extensions/api/messaging/native_message_port.h" @@ -30,7 +29,6 @@ #include "chrome/browser/tab_contents/tab_util.h" #include "components/guest_view/common/guest_view_constants.h" #include "content/public/browser/browser_thread.h" -#include "content/public/browser/notification_service.h" #include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_process_host.h" #include "content/public/browser/render_view_host.h" @@ -40,6 +38,7 @@ #include "content/public/browser/web_contents.h" #include "content/public/common/child_process_host.h" #include "extensions/browser/event_router.h" +#include "extensions/browser/extension_api_frame_id_map.h" #include "extensions/browser/extension_host.h" #include "extensions/browser/extension_registry.h" #include "extensions/browser/extension_system.h" @@ -134,10 +133,9 @@ struct MessageService::MessageChannel { struct MessageService::OpenChannelParams { int source_process_id; + int source_routing_id; scoped_ptr<base::DictionaryValue> source_tab; int source_frame_id; - int target_tab_id; - int target_frame_id; scoped_ptr<MessagePort> receiver; int receiver_port_id; std::string source_extension_id; @@ -150,10 +148,9 @@ struct MessageService::OpenChannelParams { // Takes ownership of receiver. OpenChannelParams(int source_process_id, + int source_routing_id, scoped_ptr<base::DictionaryValue> source_tab, int source_frame_id, - int target_tab_id, - int target_frame_id, MessagePort* receiver, int receiver_port_id, const std::string& source_extension_id, @@ -163,9 +160,8 @@ struct MessageService::OpenChannelParams { bool include_tls_channel_id, bool include_guest_process_info) : source_process_id(source_process_id), + source_routing_id(source_routing_id), source_frame_id(source_frame_id), - target_tab_id(target_tab_id), - target_frame_id(target_frame_id), receiver(receiver), receiver_port_id(receiver_port_id), source_extension_id(source_extension_id), @@ -197,11 +193,6 @@ static content::RenderProcessHost* GetExtensionProcess( } // namespace -content::RenderProcessHost* - MessageService::MessagePort::GetRenderProcessHost() { - return NULL; -} - // static void MessageService::AllocatePortIdPair(int* port1, int* port2) { DCHECK_CURRENTLY_ON(BrowserThread::IO); @@ -229,11 +220,6 @@ MessageService::MessageService(BrowserContext* context) LazyBackgroundTaskQueue::Get(context)), weak_factory_(this) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - - registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED, - content::NotificationService::AllBrowserContextsAndSources()); - registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED, - content::NotificationService::AllBrowserContextsAndSources()); } MessageService::~MessageService() { @@ -266,11 +252,11 @@ void MessageService::OpenChannelToExtension( bool include_tls_channel_id) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - content::RenderProcessHost* source = - content::RenderProcessHost::FromID(source_process_id); + content::RenderFrameHost* source = + content::RenderFrameHost::FromID(source_process_id, source_routing_id); if (!source) return; - BrowserContext* context = source->GetBrowserContext(); + BrowserContext* context = source->GetProcess()->GetBrowserContext(); ExtensionRegistry* registry = ExtensionRegistry::Get(context); const Extension* target_extension = @@ -341,9 +327,8 @@ void MessageService::OpenChannelToExtension( content::RenderFrameHost* rfh = content::RenderFrameHost::FromID(source_process_id, source_routing_id); - // Main frame's frameId is 0. if (rfh) - source_frame_id = !rfh->GetParent() ? 0 : source_routing_id; + source_frame_id = ExtensionApiFrameIdMap::GetFrameId(rfh); } else { // Check to see if it was a WebView making the request. // Sending messages from WebViews to extensions breaks webview isolation, @@ -352,20 +337,13 @@ void MessageService::OpenChannelToExtension( if (is_web_view && extensions::Manifest::IsComponentLocation( target_extension->location())) { include_guest_process_info = true; - auto* rfh = content::RenderFrameHost::FromID(source_process_id, - source_routing_id); - // Include |source_frame_id| so that we can retrieve the guest's frame - // routing id in OpenChannelImpl. - if (rfh) - source_frame_id = source_routing_id; } } scoped_ptr<OpenChannelParams> params(new OpenChannelParams( - source_process_id, std::move(source_tab), source_frame_id, -1, - -1, // no target_tab_id/target_frame_id for connections to extensions - nullptr, receiver_port_id, source_extension_id, target_extension_id, - source_url, channel_name, include_tls_channel_id, + source_process_id, source_routing_id, std::move(source_tab), + source_frame_id, nullptr, receiver_port_id, source_extension_id, + target_extension_id, source_url, channel_name, include_tls_channel_id, include_guest_process_info)); pending_incognito_channels_[GET_CHANNEL_ID(params->receiver_port_id)] = @@ -423,13 +401,14 @@ void MessageService::OpenChannelToNativeApp( const std::string& native_app_name) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - content::RenderProcessHost* source = - content::RenderProcessHost::FromID(source_process_id); + content::RenderFrameHost* source = + content::RenderFrameHost::FromID(source_process_id, source_routing_id); if (!source) return; #if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_LINUX) - Profile* profile = Profile::FromBrowserContext(source->GetBrowserContext()); + Profile* profile = + Profile::FromBrowserContext(source->GetProcess()->GetBrowserContext()); ExtensionService* extension_service = ExtensionSystem::Get(profile)->extension_service(); bool has_permission = false; @@ -457,14 +436,13 @@ void MessageService::OpenChannelToNativeApp( } scoped_ptr<MessageChannel> channel(new MessageChannel()); - channel->opener.reset(new ExtensionMessagePort(source, MSG_ROUTING_CONTROL, - source_extension_id)); + channel->opener.reset( + new ExtensionMessagePort(weak_factory_.GetWeakPtr(), + GET_OPPOSITE_PORT_ID(receiver_port_id), + source_extension_id, source, false)); // Get handle of the native view and pass it to the native messaging host. - content::RenderFrameHost* render_frame_host = - content::RenderFrameHost::FromID(source_process_id, source_routing_id); - gfx::NativeView native_view = - render_frame_host ? render_frame_host->GetNativeView() : nullptr; + gfx::NativeView native_view = source ? source->GetNativeView() : nullptr; std::string error = kReceivingEndDoesntExistError; scoped_ptr<NativeMessageHost> native_host = NativeMessageHost::Create( @@ -496,18 +474,21 @@ void MessageService::OpenChannelToNativeApp( } void MessageService::OpenChannelToTab(int source_process_id, + int source_routing_id, int receiver_port_id, int tab_id, int frame_id, const std::string& extension_id, const std::string& channel_name) { DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK_GE(frame_id, -1); - content::RenderProcessHost* source = - content::RenderProcessHost::FromID(source_process_id); + content::RenderFrameHost* source = + content::RenderFrameHost::FromID(source_process_id, source_routing_id); if (!source) return; - Profile* profile = Profile::FromBrowserContext(source->GetBrowserContext()); + Profile* profile = + Profile::FromBrowserContext(source->GetProcess()->GetBrowserContext()); WebContents* contents = NULL; scoped_ptr<MessagePort> receiver; @@ -520,30 +501,21 @@ void MessageService::OpenChannelToTab(int source_process_id, return; } - int receiver_routing_id; - if (frame_id > 0) { - // Positive frame ID is child frame. - int receiver_process_id = contents->GetRenderProcessHost()->GetID(); - if (!content::RenderFrameHost::FromID(receiver_process_id, frame_id)) { - // Frame does not exist. - DispatchOnDisconnect( - source, receiver_port_id, kReceivingEndDoesntExistError); - return; - } - receiver_routing_id = frame_id; - } else if (frame_id == 0) { - // Frame ID 0 is main frame. - receiver_routing_id = contents->GetMainFrame()->GetRoutingID(); - } else { - DCHECK_EQ(-1, frame_id); - // If the frame ID is not set (i.e. -1), then the channel has to be opened - // in every frame. - // TODO(robwu): Update logic so that frames that are not hosted in the main - // frame's process can also receive the port. - receiver_routing_id = MSG_ROUTING_CONTROL; + // Frame ID -1 is every frame in the tab. + bool include_child_frames = frame_id == -1; + content::RenderFrameHost* receiver_rfh = + include_child_frames + ? contents->GetMainFrame() + : ExtensionApiFrameIdMap::GetRenderFrameHostById(contents, frame_id); + if (!receiver_rfh) { + DispatchOnDisconnect( + source, receiver_port_id, kReceivingEndDoesntExistError); + return; } - receiver.reset(new ExtensionMessagePort(contents->GetRenderProcessHost(), - receiver_routing_id, extension_id)); + receiver.reset( + new ExtensionMessagePort(weak_factory_.GetWeakPtr(), + receiver_port_id, extension_id, receiver_rfh, + include_child_frames)); const Extension* extension = nullptr; if (!extension_id.empty()) { @@ -556,11 +528,11 @@ void MessageService::OpenChannelToTab(int source_process_id, scoped_ptr<OpenChannelParams> params(new OpenChannelParams( source_process_id, + source_routing_id, scoped_ptr<base::DictionaryValue>(), // Source tab doesn't make sense // for opening to tabs. -1, // If there is no tab, then there is no frame either. - tab_id, frame_id, receiver.release(), receiver_port_id, extension_id, - extension_id, + receiver.release(), receiver_port_id, extension_id, extension_id, GURL(), // Source URL doesn't make sense for opening to tabs. channel_name, false, // Connections to tabs don't get TLS channel IDs. @@ -576,46 +548,48 @@ void MessageService::OpenChannelImpl(BrowserContext* browser_context, DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_EQ(target_extension != nullptr, !params->target_extension_id.empty()); - content::RenderProcessHost* source = - content::RenderProcessHost::FromID(params->source_process_id); + content::RenderFrameHost* source = + content::RenderFrameHost::FromID(params->source_process_id, + params->source_routing_id); if (!source) return; // Closed while in flight. - if (!params->receiver || !params->receiver->GetRenderProcessHost()) { + if (!params->receiver || !params->receiver->IsValidPort()) { DispatchOnDisconnect(source, params->receiver_port_id, kReceivingEndDoesntExistError); return; } MessageChannel* channel(new MessageChannel); - channel->opener.reset(new ExtensionMessagePort(source, MSG_ROUTING_CONTROL, - params->source_extension_id)); + channel->opener.reset( + new ExtensionMessagePort(weak_factory_.GetWeakPtr(), + GET_OPPOSITE_PORT_ID(params->receiver_port_id), + params->source_extension_id, source, false)); channel->receiver.reset(params->receiver.release()); AddChannel(channel, params->receiver_port_id); + // TODO(robwu): Could |guest_process_id| and |guest_render_frame_routing_id| + // be removed? In the past extension message routing was process-based, but + // now that extensions are routed from a specific RFH, the special casing for + // guest views seems no longer necessary, because the ExtensionMessagePort can + // simply obtain the source process & frame ID directly from the RFH. int guest_process_id = content::ChildProcessHost::kInvalidUniqueID; int guest_render_frame_routing_id = MSG_ROUTING_NONE; if (params->include_guest_process_info) { guest_process_id = params->source_process_id; - guest_render_frame_routing_id = params->source_frame_id; - auto* guest_rfh = content::RenderFrameHost::FromID( - guest_process_id, guest_render_frame_routing_id); - // Reset the |source_frame_id| parameter. - params->source_frame_id = -1; + guest_render_frame_routing_id = params->source_routing_id; - DCHECK(guest_rfh == nullptr || - WebViewGuest::FromWebContents( - WebContents::FromRenderFrameHost(guest_rfh)) != nullptr); + DCHECK(WebViewGuest::FromWebContents( + WebContents::FromRenderFrameHost(source))); } // Send the connect event to the receiver. Give it the opener's port ID (the // opener has the opposite port ID). channel->receiver->DispatchOnConnect( - params->receiver_port_id, params->channel_name, - std::move(params->source_tab), params->source_frame_id, - params->target_tab_id, params->target_frame_id, guest_process_id, - guest_render_frame_routing_id, params->source_extension_id, - params->target_extension_id, params->source_url, params->tls_channel_id); + params->channel_name, std::move(params->source_tab), + params->source_frame_id, guest_process_id, guest_render_frame_routing_id, + params->source_extension_id, params->target_extension_id, + params->source_url, params->tls_channel_id); // Report the event to the event router, if the target is an extension. // @@ -662,10 +636,36 @@ void MessageService::AddChannel(MessageChannel* channel, int receiver_port_id) { pending_lazy_background_page_channels_.erase(channel_id); } +void MessageService::OpenPort(int port_id, int process_id, int routing_id) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK(!IS_OPENER_PORT_ID(port_id)); + + int channel_id = GET_CHANNEL_ID(port_id); + MessageChannelMap::iterator it = channels_.find(channel_id); + if (it == channels_.end()) + return; + + it->second->receiver->OpenPort(process_id, routing_id); +} + +void MessageService::ClosePort( + int port_id, int process_id, int routing_id, bool force_close) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + ClosePortImpl(port_id, process_id, routing_id, force_close, std::string()); +} + void MessageService::CloseChannel(int port_id, const std::string& error_message) { DCHECK_CURRENTLY_ON(BrowserThread::UI); + ClosePortImpl(port_id, content::ChildProcessHost::kInvalidUniqueID, + MSG_ROUTING_NONE, true, error_message); +} +void MessageService::ClosePortImpl(int port_id, + int process_id, + int routing_id, + bool force_close, + const std::string& error_message) { // Note: The channel might be gone already, if the other side closed first. int channel_id = GET_CHANNEL_ID(port_id); MessageChannelMap::iterator it = channels_.find(channel_id); @@ -675,12 +675,23 @@ void MessageService::CloseChannel(int port_id, if (pending != pending_lazy_background_page_channels_.end()) { lazy_background_task_queue_->AddPendingTask( pending->second.first, pending->second.second, - base::Bind(&MessageService::PendingLazyBackgroundPageCloseChannel, - weak_factory_.GetWeakPtr(), port_id, error_message)); + base::Bind(&MessageService::PendingLazyBackgroundPageClosePort, + weak_factory_.GetWeakPtr(), port_id, process_id, + routing_id, force_close, error_message)); } return; } - CloseChannelImpl(it, port_id, error_message, true); + + // The difference between closing a channel and port is that closing a port + // does not necessarily have to destroy the channel if there are multiple + // receivers, whereas closing a channel always forces all ports to be closed. + if (force_close) { + CloseChannelImpl(it, port_id, error_message, true); + } else if (IS_OPENER_PORT_ID(port_id)) { + it->second->opener->ClosePort(process_id, routing_id); + } else { + it->second->receiver->ClosePort(process_id, routing_id); + } } void MessageService::CloseChannelImpl( @@ -696,8 +707,7 @@ void MessageService::CloseChannelImpl( if (notify_other_port) { MessagePort* port = IS_OPENER_PORT_ID(closing_port_id) ? channel->receiver.get() : channel->opener.get(); - port->DispatchOnDisconnect(GET_OPPOSITE_PORT_ID(closing_port_id), - error_message); + port->DispatchOnDisconnect(error_message); } // Balance the IncrementLazyKeepaliveCount() in OpenChannelImpl. @@ -723,53 +733,6 @@ void MessageService::PostMessage(int source_port_id, const Message& message) { DispatchMessage(source_port_id, iter->second, message); } -void MessageService::Observe(int type, - const content::NotificationSource& source, - const content::NotificationDetails& details) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - - switch (type) { - case content::NOTIFICATION_RENDERER_PROCESS_TERMINATED: - case content::NOTIFICATION_RENDERER_PROCESS_CLOSED: { - content::RenderProcessHost* renderer = - content::Source<content::RenderProcessHost>(source).ptr(); - OnProcessClosed(renderer); - break; - } - default: - NOTREACHED(); - return; - } -} - -void MessageService::OnProcessClosed(content::RenderProcessHost* process) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - - // Close any channels that share this renderer. We notify the opposite - // port that its pair has closed. - for (MessageChannelMap::iterator it = channels_.begin(); - it != channels_.end(); ) { - MessageChannelMap::iterator current = it++; - - content::RenderProcessHost* opener_process = - current->second->opener->GetRenderProcessHost(); - content::RenderProcessHost* receiver_process = - current->second->receiver->GetRenderProcessHost(); - - // Only notify the other side if it has a different porocess host. - bool notify_other_port = opener_process && receiver_process && - opener_process != receiver_process; - - if (opener_process == process) { - CloseChannelImpl(current, GET_CHANNEL_OPENER_ID(current->first), - std::string(), notify_other_port); - } else if (receiver_process == process) { - CloseChannelImpl(current, GET_CHANNEL_RECEIVERS_ID(current->first), - std::string(), notify_other_port); - } - } -} - void MessageService::EnqueuePendingMessage(int source_port_id, int channel_id, const Message& message) { @@ -827,7 +790,7 @@ void MessageService::DispatchMessage(int source_port_id, MessagePort* port = IS_OPENER_PORT_ID(dest_port_id) ? channel->opener.get() : channel->receiver.get(); - port->DispatchOnMessage(message, dest_port_id); + port->DispatchOnMessage(message); } bool MessageService::MaybeAddPendingLazyBackgroundPageOpenChannelTask( @@ -882,8 +845,9 @@ void MessageService::OnOpenChannelAllowed(scoped_ptr<OpenChannelParams> params, pending_incognito_channels_.erase(pending_for_incognito); // Re-lookup the source process since it may no longer be valid. - content::RenderProcessHost* source = - content::RenderProcessHost::FromID(params->source_process_id); + content::RenderFrameHost* source = + content::RenderFrameHost::FromID(params->source_process_id, + params->source_routing_id); if (!source) { return; } @@ -894,14 +858,20 @@ void MessageService::OnOpenChannelAllowed(scoped_ptr<OpenChannelParams> params, return; } - BrowserContext* context = source->GetBrowserContext(); + BrowserContext* context = source->GetProcess()->GetBrowserContext(); // Note: we use the source's profile here. If the source is an incognito // process, we will use the incognito EPM to find the right extension process, // which depends on whether the extension uses spanning or split mode. - params->receiver.reset(new ExtensionMessagePort( - GetExtensionProcess(context, params->target_extension_id), - MSG_ROUTING_CONTROL, params->target_extension_id)); + if (content::RenderProcessHost* extension_process = + GetExtensionProcess(context, params->target_extension_id)) { + params->receiver.reset( + new ExtensionMessagePort( + weak_factory_.GetWeakPtr(), params->receiver_port_id, + params->target_extension_id, extension_process)); + } else { + params->receiver.reset(); + } // If the target requests the TLS channel id, begin the lookup for it. // The target might also be a lazy background page, checked next, but the @@ -957,13 +927,14 @@ void MessageService::GotChannelID(scoped_ptr<OpenChannelParams> params, pending_tls_channel_id_channels_.erase(pending_for_tls_channel_id); // Re-lookup the source process since it may no longer be valid. - content::RenderProcessHost* source = - content::RenderProcessHost::FromID(params->source_process_id); + content::RenderFrameHost* source = + content::RenderFrameHost::FromID(params->source_process_id, + params->source_routing_id); if (!source) { return; } - BrowserContext* context = source->GetBrowserContext(); + BrowserContext* context = source->GetProcess()->GetBrowserContext(); ExtensionRegistry* registry = ExtensionRegistry::Get(context); const Extension* target_extension = registry->enabled_extensions().GetByID(params->target_extension_id); @@ -990,20 +961,22 @@ void MessageService::PendingLazyBackgroundPageOpenChannel( if (!host) return; // TODO(mpcomplete): notify source of disconnect? - params->receiver.reset(new ExtensionMessagePort(host->render_process_host(), - MSG_ROUTING_CONTROL, - params->target_extension_id)); + params->receiver.reset( + new ExtensionMessagePort( + weak_factory_.GetWeakPtr(), params->receiver_port_id, + params->target_extension_id, host->render_process_host())); OpenChannelImpl(host->browser_context(), std::move(params), host->extension(), true /* did_enqueue */); } -void MessageService::DispatchOnDisconnect(content::RenderProcessHost* source, +void MessageService::DispatchOnDisconnect(content::RenderFrameHost* source, int port_id, const std::string& error_message) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - ExtensionMessagePort port(source, MSG_ROUTING_CONTROL, ""); - port.DispatchOnDisconnect(GET_OPPOSITE_PORT_ID(port_id), error_message); + ExtensionMessagePort port(weak_factory_.GetWeakPtr(), + GET_OPPOSITE_PORT_ID(port_id), "", source, false); + port.DispatchOnDisconnect(error_message); } void MessageService::DispatchPendingMessages(const PendingMessagesQueue& queue, diff --git a/chrome/browser/extensions/api/messaging/message_service.h b/chrome/browser/extensions/api/messaging/message_service.h index d170b97..08a3f96 100644 --- a/chrome/browser/extensions/api/messaging/message_service.h +++ b/chrome/browser/extensions/api/messaging/message_service.h @@ -16,8 +16,6 @@ #include "base/memory/weak_ptr.h" #include "base/values.h" #include "chrome/browser/extensions/api/messaging/message_property_provider.h" -#include "content/public/browser/notification_observer.h" -#include "content/public/browser/notification_registrar.h" #include "extensions/browser/api/messaging/native_message_host.h" #include "extensions/browser/browser_context_keyed_api_factory.h" #include "extensions/common/api/messaging/message.h" @@ -27,8 +25,6 @@ class Profile; namespace content { class BrowserContext; -class RenderProcessHost; -class WebContents; } namespace extensions { @@ -54,11 +50,8 @@ class LazyBackgroundTaskQueue; // // Terminology: // channel: connection between two ports -// port: an IPC::Message::Process interface and an optional routing_id (in the -// case that the port is a tab). The Process is usually either a -// RenderProcessHost or a RenderViewHost. -class MessageService : public BrowserContextKeyedAPI, - public content::NotificationObserver { +// port: One sender or receiver tied to one or more RenderFrameHost instances. +class MessageService : public BrowserContextKeyedAPI { public: // A messaging channel. Note that the opening port can be the same as the // receiver, if an extension background page wants to talk to its tab (for @@ -69,13 +62,15 @@ class MessageService : public BrowserContextKeyedAPI, class MessagePort { public: virtual ~MessagePort() {} + + // Called right before a port is connected to a channel. If false, the port + // is not used and the channel is closed. + virtual bool IsValidPort() = 0; + // Notify the port that the channel has been opened. - virtual void DispatchOnConnect(int dest_port_id, - const std::string& channel_name, + virtual void DispatchOnConnect(const std::string& channel_name, scoped_ptr<base::DictionaryValue> source_tab, int source_frame_id, - int target_tab_id, - int target_frame_id, int guest_process_id, int guest_render_frame_routing_id, const std::string& source_extension_id, @@ -85,21 +80,22 @@ class MessageService : public BrowserContextKeyedAPI, // Notify the port that the channel has been closed. If |error_message| is // non-empty, it indicates an error occurred while opening the connection. - virtual void DispatchOnDisconnect(int source_port_id, - const std::string& error_message) {} + virtual void DispatchOnDisconnect(const std::string& error_message) {} // Dispatch a message to this end of the communication. - virtual void DispatchOnMessage(const Message& message, - int target_port_id) = 0; + virtual void DispatchOnMessage(const Message& message) = 0; + + // Mark the port as opened by the specific frame. + virtual void OpenPort(int process_id, int routing_id) {} - // MessagPorts that target extensions will need to adjust their keepalive + // Close the port for the given frame. + virtual void ClosePort(int process_id, int routing_id) {} + + // MessagePorts that target extensions will need to adjust their keepalive // counts for their lazy background page. virtual void IncrementLazyKeepaliveCount() {} virtual void DecrementLazyKeepaliveCount() {} - // Get the RenderProcessHost (if any) associated with the port. - virtual content::RenderProcessHost* GetRenderProcessHost(); - protected: MessagePort() {} @@ -145,6 +141,7 @@ class MessageService : public BrowserContextKeyedAPI, // are restricted to that tab, so if there are multiple tabs in that process, // only the targeted tab will receive messages. void OpenChannelToTab(int source_process_id, + int source_routing_id, int receiver_port_id, int tab_id, int frame_id, @@ -158,6 +155,14 @@ class MessageService : public BrowserContextKeyedAPI, const std::string& source_extension_id, const std::string& native_app_name); + // Mark the given port as opened by the frame identified by + // (process_id, routing_id). + void OpenPort(int port_id, int process_id, int routing_id); + + // Closes the given port in the given frame. If this was the last frame or if + // |force_close| is true, then the other side is closed as well. + void ClosePort(int port_id, int process_id, int routing_id, bool force_close); + // Closes the message channel associated with the given port, and notifies // the other side. void CloseChannel(int port_id, const std::string& error_message); @@ -200,6 +205,12 @@ class MessageService : public BrowserContextKeyedAPI, const Extension* target_extension, bool did_enqueue); + void ClosePortImpl(int port_id, + int process_id, + int routing_id, + bool force_close, + const std::string& error_message); + void CloseChannelImpl(MessageChannelMap::iterator channel_iter, int port_id, const std::string& error_message, @@ -209,14 +220,6 @@ class MessageService : public BrowserContextKeyedAPI, // channels with the same id. void AddChannel(MessageChannel* channel, int receiver_port_id); - // content::NotificationObserver interface. - void Observe(int type, - const content::NotificationSource& source, - const content::NotificationDetails& details) override; - - // A process that might be in our list of channels has closed. - void OnProcessClosed(content::RenderProcessHost* process); - // If the channel is being opened from an incognito tab the user must allow // the connection. void OnOpenChannelAllowed(scoped_ptr<OpenChannelParams> params, bool allowed); @@ -252,11 +255,15 @@ class MessageService : public BrowserContextKeyedAPI, scoped_ptr<OpenChannelParams> params, int source_process_id, extensions::ExtensionHost* host); - void PendingLazyBackgroundPageCloseChannel(int port_id, - const std::string& error_message, - extensions::ExtensionHost* host) { + void PendingLazyBackgroundPageClosePort(int port_id, + int process_id, + int routing_id, + bool force_close, + const std::string& error_message, + extensions::ExtensionHost* host) { if (host) - CloseChannel(port_id, error_message); + ClosePortImpl(port_id, process_id, routing_id, force_close, + error_message); } void PendingLazyBackgroundPagePostMessage(int port_id, const Message& message, @@ -267,7 +274,7 @@ class MessageService : public BrowserContextKeyedAPI, // Immediate dispatches a disconnect to |source| for |port_id|. Sets source's // runtime.lastMessage to |error_message|, if any. - void DispatchOnDisconnect(content::RenderProcessHost* source, + void DispatchOnDisconnect(content::RenderFrameHost* source, int port_id, const std::string& error_message); @@ -282,7 +289,6 @@ class MessageService : public BrowserContextKeyedAPI, static const bool kServiceIsCreatedWithBrowserContext = false; static const bool kServiceIsNULLWhileTesting = true; - content::NotificationRegistrar registrar_; MessageChannelMap channels_; // A set of channel IDs waiting for TLS channel IDs to complete opening, and // any pending messages queued to be sent on those channels. This and the diff --git a/chrome/browser/extensions/api/messaging/native_message_port.cc b/chrome/browser/extensions/api/messaging/native_message_port.cc index 51e8478..dc97cf8 100644 --- a/chrome/browser/extensions/api/messaging/native_message_port.cc +++ b/chrome/browser/extensions/api/messaging/native_message_port.cc @@ -102,9 +102,16 @@ NativeMessagePort::~NativeMessagePort() { host_task_runner_->DeleteSoon(FROM_HERE, core_.release()); } -void NativeMessagePort::DispatchOnMessage( - const Message& message, - int target_port_id) { +bool NativeMessagePort::IsValidPort() { + // The native message port is immediately connected after construction, so it + // is not possible to invalidate the port between construction and connection. + // The return value doesn't matter since native messaging follows a code path + // where IsValidPort() is never called. + NOTREACHED(); + return true; +} + +void NativeMessagePort::DispatchOnMessage(const Message& message) { DCHECK(thread_checker_.CalledOnValidThread()); core_->OnMessageFromChrome(message.data); } diff --git a/chrome/browser/extensions/api/messaging/native_message_port.h b/chrome/browser/extensions/api/messaging/native_message_port.h index 438b317..a19485f 100644 --- a/chrome/browser/extensions/api/messaging/native_message_port.h +++ b/chrome/browser/extensions/api/messaging/native_message_port.h @@ -21,7 +21,8 @@ class NativeMessagePort : public MessageService::MessagePort { ~NativeMessagePort() override; // MessageService::MessagePort implementation. - void DispatchOnMessage(const Message& message, int target_port_id) override; + bool IsValidPort() override; + void DispatchOnMessage(const Message& message) override; private: class Core; diff --git a/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc b/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc index 13bf2af..635636e 100644 --- a/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc +++ b/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc @@ -26,6 +26,7 @@ #include "content/public/browser/web_contents.h" #include "content/public/common/url_constants.h" #include "extensions/browser/event_router.h" +#include "extensions/browser/extension_api_frame_id_map.h" #include "extensions/browser/view_type_utils.h" #include "net/base/net_errors.h" @@ -492,7 +493,6 @@ bool WebNavigationGetFrameFunction::RunSync() { EXTENSION_FUNCTION_VALIDATE(params.get()); int tab_id = params->details.tab_id; int frame_id = params->details.frame_id; - int process_id = params->details.process_id; SetResult(base::Value::CreateNullValue()); @@ -516,8 +516,8 @@ bool WebNavigationGetFrameFunction::RunSync() { observer->frame_navigation_state(); content::RenderFrameHost* render_frame_host = - frame_id == 0 ? web_contents->GetMainFrame() - : content::RenderFrameHost::FromID(process_id, frame_id); + ExtensionApiFrameIdMap::Get()->GetRenderFrameHostById(web_contents, + frame_id); if (!frame_navigation_state.IsValidFrame(render_frame_host)) return true; diff --git a/chrome/browser/extensions/api/web_navigation/web_navigation_api_helpers.cc b/chrome/browser/extensions/api/web_navigation/web_navigation_api_helpers.cc index a8b027a..14612ed 100644 --- a/chrome/browser/extensions/api/web_navigation/web_navigation_api_helpers.cc +++ b/chrome/browser/extensions/api/web_navigation/web_navigation_api_helpers.cc @@ -21,6 +21,7 @@ #include "content/public/browser/render_view_host.h" #include "content/public/browser/web_contents.h" #include "extensions/browser/event_router.h" +#include "extensions/browser/extension_api_frame_id_map.h" #include "extensions/common/event_filtering_info.h" #include "net/base/net_errors.h" #include "ui/base/page_transition_types.h" @@ -62,15 +63,10 @@ void DispatchEvent(content::BrowserContext* browser_context, } // namespace int GetFrameId(content::RenderFrameHost* frame_host) { - if (!frame_host) - return -1; - return !frame_host->GetParent() ? 0 : frame_host->GetRoutingID(); + return ExtensionApiFrameIdMap::GetFrameId(frame_host); } // Constructs and dispatches an onBeforeNavigate event. -// TODO(dcheng): Is the parent process ID needed here? http://crbug.com/393640 -// Collisions are probably possible... but maybe this won't ever happen because -// of the SiteInstance grouping policies. void DispatchOnBeforeNavigate(content::WebContents* web_contents, content::RenderFrameHost* frame_host, const GURL& validated_url) { diff --git a/chrome/browser/extensions/api/web_navigation/web_navigation_apitest.cc b/chrome/browser/extensions/api/web_navigation/web_navigation_apitest.cc index e2c2379..46d1020d 100644 --- a/chrome/browser/extensions/api/web_navigation/web_navigation_apitest.cc +++ b/chrome/browser/extensions/api/web_navigation/web_navigation_apitest.cc @@ -44,6 +44,7 @@ #include "content/public/common/resource_type.h" #include "content/public/common/url_constants.h" #include "content/public/test/browser_test_utils.h" +#include "content/public/test/test_utils.h" #include "extensions/browser/extension_system.h" #include "extensions/common/switches.h" #include "extensions/test/result_catcher.h" @@ -739,6 +740,12 @@ IN_PROC_BROWSER_TEST_F(WebNavigationApiTest, CrossProcessHistory) { << message_; } +IN_PROC_BROWSER_TEST_F(WebNavigationApiTest, CrossProcessIframe) { + content::IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess()); + ASSERT_TRUE(StartEmbeddedTestServer()); + ASSERT_TRUE(RunExtensionTest("webnavigation/crossProcessIframe")) << message_; +} + // TODO(jam): http://crbug.com/350550 #if !(defined(OS_CHROMEOS) && defined(ADDRESS_SANITIZER)) IN_PROC_BROWSER_TEST_F(WebNavigationApiTest, Crash) { diff --git a/chrome/browser/extensions/extension_messages_apitest.cc b/chrome/browser/extensions/extension_messages_apitest.cc index eb2c6ba..e6e80c4 100644 --- a/chrome/browser/extensions/extension_messages_apitest.cc +++ b/chrome/browser/extensions/extension_messages_apitest.cc @@ -39,6 +39,8 @@ #include "extensions/common/api/runtime.h" #include "extensions/common/extension_builder.h" #include "extensions/common/value_builder.h" +#include "extensions/test/extension_test_message_listener.h" +#include "extensions/test/result_catcher.h" #include "net/cert/asn1_util.h" #include "net/cert/jwk_serializer.h" #include "net/dns/mock_host_resolver.h" @@ -118,6 +120,22 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Messaging) { ASSERT_TRUE(RunExtensionTest("messaging/connect")) << message_; } +IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MessagingCrash) { + ASSERT_TRUE(StartEmbeddedTestServer()); + ExtensionTestMessageListener ready_to_crash("ready_to_crash", true); + ASSERT_TRUE(LoadExtension( + test_data_dir_.AppendASCII("messaging/connect_crash"))); + ui_test_utils::NavigateToURL( + browser(), embedded_test_server()->GetURL("/extensions/test_file.html")); + content::WebContents* tab = + browser()->tab_strip_model()->GetActiveWebContents(); + EXPECT_TRUE(ready_to_crash.WaitUntilSatisfied()); + + ResultCatcher catcher; + CrashTab(tab); + EXPECT_TRUE(catcher.GetNextResult()); +} + // Tests that message passing from one extension to another works. IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MessagingExternal) { ASSERT_TRUE(LoadExtension( diff --git a/chrome/browser/renderer_host/chrome_extension_message_filter.cc b/chrome/browser/renderer_host/chrome_extension_message_filter.cc index 921b225..59412fc 100644 --- a/chrome/browser/renderer_host/chrome_extension_message_filter.cc +++ b/chrome/browser/renderer_host/chrome_extension_message_filter.cc @@ -86,10 +86,11 @@ bool ChromeExtensionMessageFilter::OnMessageReceived( IPC_MESSAGE_HANDLER(ExtensionHostMsg_OpenChannelToTab, OnOpenChannelToTab) IPC_MESSAGE_HANDLER(ExtensionHostMsg_OpenChannelToNativeApp, OnOpenChannelToNativeApp) + IPC_MESSAGE_HANDLER(ExtensionHostMsg_OpenMessagePort, OnOpenMessagePort) + IPC_MESSAGE_HANDLER(ExtensionHostMsg_CloseMessagePort, OnCloseMessagePort) IPC_MESSAGE_HANDLER(ExtensionHostMsg_PostMessage, OnPostMessage) IPC_MESSAGE_HANDLER_DELAY_REPLY(ExtensionHostMsg_GetMessageBundle, OnGetExtMessageBundle) - IPC_MESSAGE_HANDLER(ExtensionHostMsg_CloseChannel, OnExtensionCloseChannel) IPC_MESSAGE_HANDLER(ExtensionHostMsg_AddAPIActionToActivityLog, OnAddAPIActionToExtensionActivityLog); IPC_MESSAGE_HANDLER(ExtensionHostMsg_AddDOMActionToActivityLog, @@ -105,8 +106,9 @@ bool ChromeExtensionMessageFilter::OnMessageReceived( void ChromeExtensionMessageFilter::OverrideThreadForMessage( const IPC::Message& message, BrowserThread::ID* thread) { switch (message.type()) { + case ExtensionHostMsg_OpenMessagePort::ID: + case ExtensionHostMsg_CloseMessagePort::ID: case ExtensionHostMsg_PostMessage::ID: - case ExtensionHostMsg_CloseChannel::ID: case ExtensionHostMsg_AddAPIActionToActivityLog::ID: case ExtensionHostMsg_AddDOMActionToActivityLog::ID: case ExtensionHostMsg_AddEventToActivityLog::ID: @@ -194,6 +196,7 @@ void ChromeExtensionMessageFilter::OpenChannelToNativeAppOnUIThread( } void ChromeExtensionMessageFilter::OnOpenChannelToTab( + int routing_id, const ExtensionMsg_TabTargetConnectionInfo& info, const std::string& extension_id, const std::string& channel_name, @@ -204,12 +207,13 @@ void ChromeExtensionMessageFilter::OnOpenChannelToTab( BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, base::Bind(&ChromeExtensionMessageFilter::OpenChannelToTabOnUIThread, - this, render_process_id_, port2_id, info, extension_id, - channel_name)); + this, render_process_id_, routing_id, port2_id, info, + extension_id, channel_name)); } void ChromeExtensionMessageFilter::OpenChannelToTabOnUIThread( int source_process_id, + int source_routing_id, int receiver_port_id, const ExtensionMsg_TabTargetConnectionInfo& info, const std::string& extension_id, @@ -218,6 +222,7 @@ void ChromeExtensionMessageFilter::OpenChannelToTabOnUIThread( if (profile_) { extensions::MessageService::Get(profile_) ->OpenChannelToTab(source_process_id, + source_routing_id, receiver_port_id, info.tab_id, info.frame_id, @@ -226,6 +231,25 @@ void ChromeExtensionMessageFilter::OpenChannelToTabOnUIThread( } } +void ChromeExtensionMessageFilter::OnOpenMessagePort(int routing_id, + int port_id) { + if (!profile_) + return; + + extensions::MessageService::Get(profile_)->OpenPort( + port_id, render_process_id_, routing_id); +} + +void ChromeExtensionMessageFilter::OnCloseMessagePort(int routing_id, + int port_id, + bool force_close) { + if (!profile_) + return; + + extensions::MessageService::Get(profile_)->ClosePort( + port_id, render_process_id_, routing_id, force_close); +} + void ChromeExtensionMessageFilter::OnPostMessage( int port_id, const extensions::Message& message) { @@ -261,21 +285,6 @@ void ChromeExtensionMessageFilter::OnGetExtMessageBundleOnBlockingPool( Send(reply_msg); } -void ChromeExtensionMessageFilter::OnExtensionCloseChannel( - int port_id, - const std::string& error_message) { - if (!profile_) - return; - - if (!content::RenderProcessHost::FromID(render_process_id_)) - return; // To guard against crash in browser_tests shutdown. - - extensions::MessageService* message_service = - extensions::MessageService::Get(profile_); - if (message_service) - message_service->CloseChannel(port_id, error_message); -} - void ChromeExtensionMessageFilter::OnAddAPIActionToExtensionActivityLog( const std::string& extension_id, const ExtensionHostMsg_APIActionOrEvent_Params& params) { diff --git a/chrome/browser/renderer_host/chrome_extension_message_filter.h b/chrome/browser/renderer_host/chrome_extension_message_filter.h index 04762a9..50b958e 100644 --- a/chrome/browser/renderer_host/chrome_extension_message_filter.h +++ b/chrome/browser/renderer_host/chrome_extension_message_filter.h @@ -72,23 +72,26 @@ class ChromeExtensionMessageFilter : public content::BrowserMessageFilter, int receiver_port_id, const std::string& source_extension_id, const std::string& native_app_name); - void OnOpenChannelToTab(const ExtensionMsg_TabTargetConnectionInfo& info, + void OnOpenChannelToTab(int routing_id, + const ExtensionMsg_TabTargetConnectionInfo& info, const std::string& extension_id, const std::string& channel_name, int* port_id); void OpenChannelToTabOnUIThread( int source_process_id, + int source_routing_id, int receiver_port_id, const ExtensionMsg_TabTargetConnectionInfo& info, const std::string& extension_id, const std::string& channel_name); + void OnOpenMessagePort(int routing_id, int port_id); + void OnCloseMessagePort(int routing_id, int port_id, bool force_close); void OnPostMessage(int port_id, const extensions::Message& message); void OnGetExtMessageBundle(const std::string& extension_id, IPC::Message* reply_msg); void OnGetExtMessageBundleOnBlockingPool( const std::string& extension_id, IPC::Message* reply_msg); - void OnExtensionCloseChannel(int port_id, const std::string& error_message); void OnAddAPIActionToExtensionActivityLog( const std::string& extension_id, const ExtensionHostMsg_APIActionOrEvent_Params& params); diff --git a/chrome/common/extensions/api/web_navigation.json b/chrome/common/extensions/api/web_navigation.json index fb79b82..fa32a6d 100644 --- a/chrome/common/extensions/api/web_navigation.json +++ b/chrome/common/extensions/api/web_navigation.json @@ -31,7 +31,12 @@ "description": "Information about the frame to retrieve information about.", "properties": { "tabId": { "type": "integer", "minimum": 0, "description": "The ID of the tab in which the frame is." }, - "processId": {"type": "integer", "description": "The ID of the process runs the renderer for this tab."}, + "processId": { + "type": "integer", + "optional": true, + "deprecated": "Frames are now uniquely identified by their tab ID and frame ID; the process ID is no longer needed and therefore ignored.", + "description": "The ID of the process that runs the renderer for this tab." + }, "frameId": { "type": "integer", "minimum": 0, "description": "The ID of the frame in the given tab." } } }, @@ -90,7 +95,7 @@ }, "processId": { "type": "integer", - "description": "The ID of the process runs the renderer for this tab." + "description": "The ID of the process that runs the renderer for this frame." }, "frameId": { "type": "integer", @@ -132,7 +137,7 @@ "properties": { "tabId": {"type": "integer", "description": "The ID of the tab in which the navigation is about to occur."}, "url": {"type": "string"}, - "processId": {"type": "integer", "description": "The ID of the process runs the renderer for this tab."}, + "processId": {"type": "integer", "description": "The ID of the process that runs the renderer for this frame."}, "frameId": {"type": "integer", "description": "0 indicates the navigation happens in the tab content window; a positive value indicates navigation in a subframe. Frame IDs are unique for a given tab and process."}, "parentFrameId": {"type": "integer", "description": "ID of frame that wraps the frame. Set to -1 of no parent frame exists."}, "timeStamp": {"type": "number", "description": "The time when the browser was about to start the navigation, in milliseconds since the epoch."} @@ -159,7 +164,7 @@ "properties": { "tabId": {"type": "integer", "description": "The ID of the tab in which the navigation occurs."}, "url": {"type": "string"}, - "processId": {"type": "integer", "description": "The ID of the process runs the renderer for this tab."}, + "processId": {"type": "integer", "description": "The ID of the process that runs the renderer for this frame."}, "frameId": {"type": "integer", "description": "0 indicates the navigation happens in the tab content window; a positive value indicates navigation in a subframe. Frame IDs are unique within a tab."}, "transitionType": {"$ref": "TransitionType", "description": "Cause of the navigation."}, "transitionQualifiers": {"type": "array", "description": "A list of transition qualifiers.", "items": {"$ref": "TransitionQualifier"}}, @@ -187,7 +192,7 @@ "properties": { "tabId": {"type": "integer", "description": "The ID of the tab in which the navigation occurs."}, "url": {"type": "string"}, - "processId": {"type": "integer", "description": "The ID of the process runs the renderer for this tab."}, + "processId": {"type": "integer", "description": "The ID of the process that runs the renderer for this frame."}, "frameId": {"type": "integer", "description": "0 indicates the navigation happens in the tab content window; a positive value indicates navigation in a subframe. Frame IDs are unique within a tab."}, "timeStamp": {"type": "number", "description": "The time when the page's DOM was fully constructed, in milliseconds since the epoch."} } @@ -213,7 +218,7 @@ "properties": { "tabId": {"type": "integer", "description": "The ID of the tab in which the navigation occurs."}, "url": {"type": "string"}, - "processId": {"type": "integer", "description": "The ID of the process runs the renderer for this tab."}, + "processId": {"type": "integer", "description": "The ID of the process that runs the renderer for this frame."}, "frameId": {"type": "integer", "description": "0 indicates the navigation happens in the tab content window; a positive value indicates navigation in a subframe. Frame IDs are unique within a tab."}, "timeStamp": {"type": "number", "description": "The time when the document finished loading, in milliseconds since the epoch."} } @@ -239,7 +244,7 @@ "properties": { "tabId": {"type": "integer", "description": "The ID of the tab in which the navigation occurs."}, "url": {"type": "string"}, - "processId": {"type": "integer", "description": "The ID of the process runs the renderer for this tab."}, + "processId": {"type": "integer", "description": "The ID of the process that runs the renderer for this frame."}, "frameId": {"type": "integer", "description": "0 indicates the navigation happens in the tab content window; a positive value indicates navigation in a subframe. Frame IDs are unique within a tab."}, "error": {"type": "string", "description": "The error description."}, "timeStamp": {"type": "number", "description": "The time when the error occurred, in milliseconds since the epoch."} @@ -265,7 +270,7 @@ "name": "details", "properties": { "sourceTabId": {"type": "integer", "description": "The ID of the tab in which the navigation is triggered."}, - "sourceProcessId": {"type": "integer", "description": "The ID of the process runs the renderer for the source tab."}, + "sourceProcessId": {"type": "integer", "description": "The ID of the process that runs the renderer for the source frame."}, "sourceFrameId": {"type": "integer", "description": "The ID of the frame with sourceTabId in which the navigation is triggered. 0 indicates the main frame."}, "url": {"type": "string", "description": "The URL to be opened in the new window."}, "tabId": {"type": "integer", "description": "The ID of the tab in which the url is opened"}, @@ -293,7 +298,7 @@ "properties": { "tabId": {"type": "integer", "description": "The ID of the tab in which the navigation occurs."}, "url": {"type": "string"}, - "processId": {"type": "integer", "description": "The ID of the process runs the renderer for this tab."}, + "processId": {"type": "integer", "description": "The ID of the process that runs the renderer for this frame."}, "frameId": {"type": "integer", "description": "0 indicates the navigation happens in the tab content window; a positive value indicates navigation in a subframe. Frame IDs are unique within a tab."}, "transitionType": {"$ref": "TransitionType", "description": "Cause of the navigation."}, "transitionQualifiers": {"type": "array", "description": "A list of transition qualifiers.", "items": {"$ref": "TransitionQualifier"}}, @@ -337,7 +342,7 @@ "properties": { "tabId": {"type": "integer", "description": "The ID of the tab in which the navigation occurs."}, "url": {"type": "string"}, - "processId": {"type": "integer", "description": "The ID of the process runs the renderer for this tab."}, + "processId": {"type": "integer", "description": "The ID of the process that runs the renderer for this frame."}, "frameId": {"type": "integer", "description": "0 indicates the navigation happens in the tab content window; a positive value indicates navigation in a subframe. Frame IDs are unique within a tab."}, "transitionType": {"$ref": "TransitionType", "description": "Cause of the navigation."}, "transitionQualifiers": {"type": "array", "description": "A list of transition qualifiers.", "items": {"$ref": "TransitionQualifier"}}, diff --git a/chrome/common/extensions/docs/templates/intros/webNavigation.html b/chrome/common/extensions/docs/templates/intros/webNavigation.html index 36e3136..6dc666e 100644 --- a/chrome/common/extensions/docs/templates/intros/webNavigation.html +++ b/chrome/common/extensions/docs/templates/intros/webNavigation.html @@ -103,21 +103,23 @@ extension (via <code>(new Date()).getTime()</code>, for instance) might give unexpected results. </p> -<h2 id="frame_ids">A note about frame and process IDs</h2> +<h2 id="frame_ids">A note about frame IDs</h2> <p> Frames within a tab can be identified by a frame ID. The frame ID of the main frame is always 0, the ID of child frames is a positive number. Once a document is constructed in a frame, its frame ID remains constant during the lifetime of -the document. +the document. As of Chrome 49, this ID is also constant for the lifetime of the +frame (across multiple navigations). </p> <p> Due to the multi-process nature of Chrome, a tab might use different processes to render the source and destination of a web page. Therefore, if a navigation takes place in a new process, you might receive events both from the new and the old page until the new navigation is committed (i.e. the -<code>onCommitted</code> event is send for the new main frame). Because frame -IDs are only unique for a given process, the webNavigation events include a -process ID, so you can still determine which frame a navigation came from. +<code>onCommitted</code> event is send for the new main frame). In other words, +it is possible to have more than one pending sequence of webNavigation events +with the same <code>frameId</code>. The sequences can be distinguished by the +<code>processId</code> key. </p> <p> Also note that during a provisional load the process might be switched several diff --git a/chrome/renderer/extensions/tabs_custom_bindings.cc b/chrome/renderer/extensions/tabs_custom_bindings.cc index b9d5d24..2459f96 100644 --- a/chrome/renderer/extensions/tabs_custom_bindings.cc +++ b/chrome/renderer/extensions/tabs_custom_bindings.cc @@ -49,7 +49,8 @@ void TabsCustomBindings::OpenChannelToTab( std::string channel_name = *v8::String::Utf8Value(args[3]); int port_id = -1; render_frame->Send(new ExtensionHostMsg_OpenChannelToTab( - info, extension_id, channel_name, &port_id)); + render_frame->GetRoutingID(), info, extension_id, channel_name, + &port_id)); args.GetReturnValue().Set(static_cast<int32_t>(port_id)); } diff --git a/chrome/test/data/extensions/api_test/messaging/connect/frame.js b/chrome/test/data/extensions/api_test/messaging/connect/frame.js index b7a3c4f..a6f9df1 100644 --- a/chrome/test/data/extensions/api_test/messaging/connect/frame.js +++ b/chrome/test/data/extensions/api_test/messaging/connect/frame.js @@ -9,9 +9,16 @@ chrome.runtime.onConnect.addListener(function(port) { // This number is used in test.js to identify messages from this frame. var test_id = location.search.slice(-1); port.postMessage('from_' + test_id); + } else if (msg.testConnectChildFrameAndNavigate) { + location.search = '?testConnectChildFrameAndNavigateDone'; } }); }); // continuation of testSendMessageFromFrame() -chrome.runtime.sendMessage({frameUrl: location.href}); +if (location.search.lastIndexOf('?testSendMessageFromFrame', 0) === 0) { + chrome.runtime.sendMessage({frameUrl: location.href}); +} else if (location.search === '?testConnectChildFrameAndNavigateSetup') { + // continuation of connectChildFrameAndNavigate() 1/2 + chrome.runtime.sendMessage('testConnectChildFrameAndNavigateSetupDone'); +} diff --git a/chrome/test/data/extensions/api_test/messaging/connect/manifest.json b/chrome/test/data/extensions/api_test/messaging/connect/manifest.json index abb898b..e9af6db 100644 --- a/chrome/test/data/extensions/api_test/messaging/connect/manifest.json +++ b/chrome/test/data/extensions/api_test/messaging/connect/manifest.json @@ -15,7 +15,10 @@ }, { "all_frames": true, - "matches": ["http://*/*?testSendMessageFromFrame*"], + "matches": [ + "http://*/*?testSendMessageFromFrame*", + "http://*/*?testConnectChildFrameAndNavigate*" + ], "js": ["frame.js"] } ] diff --git a/chrome/test/data/extensions/api_test/messaging/connect/page.js b/chrome/test/data/extensions/api_test/messaging/connect/page.js index b8a88a3..bf122e7 100644 --- a/chrome/test/data/extensions/api_test/messaging/connect/page.js +++ b/chrome/test/data/extensions/api_test/messaging/connect/page.js @@ -19,7 +19,7 @@ Object.prototype.toJSON = function() { }; // For complex connect tests. -chrome.runtime.onConnect.addListener(function(port) { +chrome.runtime.onConnect.addListener(function onConnect(port) { console.log('connected'); port.onMessage.addListener(function(msg) { console.log('got ' + msg); @@ -35,6 +35,10 @@ chrome.runtime.onConnect.addListener(function(port) { port.postMessage('from_main'); } else if (msg.testDisconnect) { port.disconnect(); + } else if (msg.testConnectChildFrameAndNavigateSetup) { + chrome.runtime.onConnect.removeListener(onConnect); + chrome.test.assertFalse(chrome.runtime.onConnect.hasListeners()); + testConnectChildFrameAndNavigateSetup(); } else if (msg.testDisconnectOnClose) { window.location = "about:blank"; } else if (msg.testPortName) { @@ -81,6 +85,17 @@ function testSendMessageFromFrame() { } } +function testConnectChildFrameAndNavigateSetup() { + var frames = document.querySelectorAll('iframe'); + for (var i = 0; i < frames.length; ++i) { + frames[i].remove(); + } + var f = document.createElement('iframe'); + f.src = '?testConnectChildFrameAndNavigateSetup'; + document.body.appendChild(f); + // Test will continue in frame.js +} + // Tests sendMessage to an invalid extension. function testSendMessageFromTabError() { // try sending a request to a bad extension id diff --git a/chrome/test/data/extensions/api_test/messaging/connect/test.js b/chrome/test/data/extensions/api_test/messaging/connect/test.js index 50dbb8c..356f3d6 100644 --- a/chrome/test/data/extensions/api_test/messaging/connect/test.js +++ b/chrome/test/data/extensions/api_test/messaging/connect/test.js @@ -260,6 +260,21 @@ chrome.test.getConfig(function(config) { } }, + // Tests that reloading a child frame disconnects the port if it was the + // only recipient of the port (i.e. no onConnect in main frame). + function connectChildFrameAndNavigate() { + listenOnce(chrome.runtime.onMessage, function(msg) { + chrome.test.assertEq('testConnectChildFrameAndNavigateSetupDone', msg); + // Now we have set up a frame and ensured that there is no onConnect + // handler in the main frame. Run the actual test: + var port = chrome.tabs.connect(testTab.id); + listenOnce(port.onDisconnect, function() {}); + port.postMessage({testConnectChildFrameAndNavigate: true}); + }); + chrome.tabs.connect(testTab.id) + .postMessage({testConnectChildFrameAndNavigateSetup: true}); + }, + // Tests that we get the disconnect event when the tab context closes. function disconnectOnClose() { var port = chrome.tabs.connect(testTab.id); @@ -300,7 +315,6 @@ chrome.test.getConfig(function(config) { 'Could not establish connection. Receiving end does not exist.', function() { stopFailing(); - chrome.test.succeed(); } )); }, @@ -330,14 +344,12 @@ chrome.test.getConfig(function(config) { // chrome.test.assertEq(location.href, sender.url); // setTimeout(function() { // stopFailing(); - // chrome.test.succeed(); // }, 0); // } // ); // // chrome.runtime.sendMessage('ping'); // }, - ]); }); @@ -347,10 +359,12 @@ function connectToTabWithFrameId(frameId, expectedMessages) { }); var messages = []; var isDone = false; - listenForever(port.onMessage, function(message) { - if (isDone) // Should not get any messages after completing the test. + port.onMessage.addListener(function(message) { + if (isDone) { // Should not get any messages after completing the test. chrome.test.fail( 'Unexpected message from port to frame ' + frameId + ': ' + message); + return; + } messages.push(message); isDone = messages.length == expectedMessages.length; @@ -359,7 +373,7 @@ function connectToTabWithFrameId(frameId, expectedMessages) { chrome.test.succeed(); } }); - listenOnce(port.onDisconnect, function() { + port.onDisconnect.addListener(function() { if (!isDone) // The event should never be triggered when we expect messages. chrome.test.fail('Unexpected disconnect from port to frame ' + frameId); }); diff --git a/chrome/test/data/extensions/api_test/messaging/connect_crash/manifest.json b/chrome/test/data/extensions/api_test/messaging/connect_crash/manifest.json new file mode 100644 index 0000000..7660f3e --- /dev/null +++ b/chrome/test/data/extensions/api_test/messaging/connect_crash/manifest.json @@ -0,0 +1,13 @@ +{ + "name": "connect_crash", + "version": "1.0", + "manifest_version": 2, + "description": "Tests that the port is disconnect if the tab crashes.", + "background": { + "scripts": ["test.js"] + }, + "content_scripts": [{ + "matches": ["*://*/*"], + "js": ["page.js"] + }] +} diff --git a/chrome/test/data/extensions/api_test/messaging/connect_crash/page.js b/chrome/test/data/extensions/api_test/messaging/connect_crash/page.js new file mode 100644 index 0000000..b3fc833 --- /dev/null +++ b/chrome/test/data/extensions/api_test/messaging/connect_crash/page.js @@ -0,0 +1,24 @@ +// Copyright 2015 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. + +var port = chrome.runtime.connect(); +port.onDisconnect.addListener(function() { + chrome.test.fail('onDisconnect should not be triggered because the ' + + 'background page exists and the tab should have been crashed'); +}); + +var ref; +chrome.runtime.onMessage.addListener(function(msg, sender, sendResponse) { + chrome.test.assertEq('Rob says hi', msg); + port.postMessage('is_ready_to_crash'); + // Keep the callback around to avoid test flakiness due to GC. + ref = sendResponse; + + // TODO(robwu): Remove the following line once crbug.com/439780 is fixed. + // (the response callback is not automatically invoked when the tab crashes). + sendResponse(); + + // Keep the port open - do not send a response. + return true; +}); diff --git a/chrome/test/data/extensions/api_test/messaging/connect_crash/test.js b/chrome/test/data/extensions/api_test/messaging/connect_crash/test.js new file mode 100644 index 0000000..dbb865d --- /dev/null +++ b/chrome/test/data/extensions/api_test/messaging/connect_crash/test.js @@ -0,0 +1,27 @@ +// Copyright 2015 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. + +chrome.runtime.onConnect.addListener(function(port) { + var is_ready_to_crash = false; + var succeed1 = chrome.test.callbackAdded(); + var succeed2 = chrome.test.callbackAdded(); + + port.onMessage.addListener(function(msg) { + chrome.test.assertEq('is_ready_to_crash', msg); + is_ready_to_crash = true; + chrome.test.sendMessage('ready_to_crash'); + // Now the browser test should kill the tab, and the port should be closed. + }); + port.onDisconnect.addListener(function() { + chrome.test.log('port.onDisconnect was triggered.'); + chrome.test.assertTrue(is_ready_to_crash); + succeed1(); + }); + + chrome.tabs.sendMessage(port.sender.tab.id, 'Rob says hi', function() { + chrome.test.log('tab.sendMessage\'s response callback was invoked'); + chrome.test.assertNoLastError(); + succeed2(); + }); +}); diff --git a/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/frame.html b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/frame.html new file mode 100644 index 0000000..8c58ffd --- /dev/null +++ b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/frame.html @@ -0,0 +1,3 @@ +<body> +<script src="frame.js"></script> +</body> diff --git a/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/frame.js b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/frame.js new file mode 100644 index 0000000..46e3f80 --- /dev/null +++ b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/frame.js @@ -0,0 +1,18 @@ +// Copyright 2015 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. + +onload = function() { + setTimeout(function() { + if (location.hostname === 'a.com') { + // Notify the parent frame, so they can navigate us. + parent.postMessage('a.com: go to b.com', location.ancestorOrigins[0]); + } else if (location.hostname === 'b.com') { + // Trigger a navigation to a site in another process. + location.hostname = 'c.com'; + } else { + console.assert(location.hostname === 'c.com'); + // Done. + } + }, 0); +}; diff --git a/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/framework.js b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/framework.js new file mode 100644 index 0000000..b380ccb --- /dev/null +++ b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/framework.js @@ -0,0 +1,263 @@ +// Copyright 2015 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. + +var deepEq = chrome.test.checkDeepEq; +var expectedEventData; +var expectedEventOrder; +var capturedEventData; +var nextFrameId; +var frameIds; +var nextTabId; +var tabIds; +var nextProcessId; +var processIds; +var initialized = false; + +var debug = false; + +function deepCopy(obj) { + if (obj === null) + return null; + if (typeof(obj) != 'object') + return obj; + if (Array.isArray(obj)) { + var tmp_array = new Array; + for (var i = 0; i < obj.length; i++) { + tmp_array.push(deepCopy(obj[i])); + } + return tmp_array; + } + + var tmp_object = {} + for (var p in obj) { + tmp_object[p] = deepCopy(obj[p]); + } + return tmp_object; +} + +// data: array of expected events, each one is a dictionary: +// { label: "<unique identifier>", +// event: "<webnavigation event type>", +// details: { <expected details of the event> } +// } +// order: an array of sequences, e.g. [ ["a", "b", "c"], ["d", "e"] ] means that +// event with label "a" needs to occur before event with label "b". The +// relative order of "a" and "d" does not matter. +function expect(data, order) { + expectedEventData = data; + capturedEventData = []; + expectedEventOrder = order; + nextFrameId = 1; + frameIds = {}; + nextTabId = 0; + tabIds = {}; + nextProcessId = 0; + processIds = {} + initListeners(); +} + +function checkExpectations() { + if (capturedEventData.length < expectedEventData.length) { + return; + } + if (capturedEventData.length > expectedEventData.length) { + chrome.test.fail("Recorded too many events. " + + JSON.stringify(capturedEventData)); + } + // We have ensured that capturedEventData contains exactly the same elements + // as expectedEventData. Now we need to verify the ordering. + // Step 1: build positions such that + // position[<event-label>]=<position of this event in capturedEventData> + var curPos = 0; + var positions = {}; + capturedEventData.forEach(function (event) { + chrome.test.assertTrue(event.hasOwnProperty("label")); + positions[event.label] = curPos; + curPos++; + }); + // Step 2: check that elements arrived in correct order + expectedEventOrder.forEach(function (order) { + var previousLabel = undefined; + order.forEach(function (label) { + if (previousLabel === undefined) { + previousLabel = label; + return; + } + chrome.test.assertTrue(positions[previousLabel] < positions[label], + "Event " + previousLabel + " is supposed to arrive before " + + label + "."); + previousLabel = label; + }); + }); + chrome.test.succeed(); +} + +function captureEvent(name, details) { + if ('url' in details) { + // Skip about:blank navigations + if (details.url == 'about:blank') { + return; + } + // Strip query parameter as it is hard to predict. + details.url = details.url.replace(new RegExp('\\?[^#]*'), ''); + } + // normalize details. + if ('timeStamp' in details) { + details.timeStamp = 0; + } + if (('frameId' in details) && (details.frameId != 0)) { + if (frameIds[details.frameId] === undefined) { + frameIds[details.frameId] = nextFrameId++; + } + details.frameId = frameIds[details.frameId]; + } + if (('parentFrameId' in details) && (details.parentFrameId > 0)) { + if (frameIds[details.parentFrameId] === undefined) { + frameIds[details.parentFrameId] = nextFrameId++; + } + details.parentFrameId = frameIds[details.parentFrameId]; + } + if (('sourceFrameId' in details) && (details.sourceFrameId != 0)) { + if (frameIds[details.sourceFrameId] === undefined) { + frameIds[details.sourceFrameId] = nextFrameId++; + } + details.sourceFrameId = frameIds[details.sourceFrameId]; + } + if ('tabId' in details) { + if (tabIds[details.tabId] === undefined) { + tabIds[details.tabId] = nextTabId++; + } + details.tabId = tabIds[details.tabId]; + } + if ('sourceTabId' in details) { + if (tabIds[details.sourceTabId] === undefined) { + tabIds[details.sourceTabId] = nextTabId++; + } + details.sourceTabId = tabIds[details.sourceTabId]; + } + if ('replacedTabId' in details) { + if (tabIds[details.replacedTabId] === undefined) { + tabIds[details.replacedTabId] = nextTabId++; + } + details.replacedTabId = tabIds[details.replacedTabId]; + } + if ('processId' in details) { + if (processIds[details.processId] === undefined) { + processIds[details.processId] = nextProcessId++; + } + details.processId = processIds[details.processId]; + } + if ('sourceProcessId' in details) { + if (processIds[details.sourceProcessId] === undefined) { + processIds[details.sourceProcessId] = nextProcessId++; + } + details.sourceProcessId = processIds[details.sourceProcessId]; + } + + if (debug) + console.log("Received event '" + name + "':" + JSON.stringify(details)); + + // find |details| in expectedEventData + var found = false; + var label = undefined; + expectedEventData.forEach(function (exp) { + if (exp.event == name) { + var exp_details; + var alt_details; + if ('transitionQualifiers' in exp.details) { + var idx = exp.details['transitionQualifiers'].indexOf( + 'maybe_client_redirect'); + if (idx >= 0) { + exp_details = deepCopy(exp.details); + exp_details['transitionQualifiers'].splice(idx, 1); + alt_details = deepCopy(exp_details); + alt_details['transitionQualifiers'].push('client_redirect'); + } else { + exp_details = exp.details; + alt_details = exp.details; + } + } else { + exp_details = exp.details; + alt_details = exp.details; + } + if (deepEq(exp_details, details) || deepEq(alt_details, details)) { + if (!found) { + found = true; + label = exp.label; + exp.event = undefined; + } + } + } + }); + if (!found) { + chrome.test.fail("Received unexpected event '" + name + "':" + + JSON.stringify(details)); + } + capturedEventData.push({label: label, event: name, details: details}); + checkExpectations(); +} + +function initListeners() { + if (initialized) + return; + initialized = true; + chrome.webNavigation.onBeforeNavigate.addListener( + function(details) { + captureEvent("onBeforeNavigate", details); + }); + chrome.webNavigation.onCommitted.addListener( + function(details) { + captureEvent("onCommitted", details); + }); + chrome.webNavigation.onDOMContentLoaded.addListener( + function(details) { + captureEvent("onDOMContentLoaded", details); + }); + chrome.webNavigation.onCompleted.addListener( + function(details) { + captureEvent("onCompleted", details); + }); + chrome.webNavigation.onCreatedNavigationTarget.addListener( + function(details) { + captureEvent("onCreatedNavigationTarget", details); + }); + chrome.webNavigation.onReferenceFragmentUpdated.addListener( + function(details) { + captureEvent("onReferenceFragmentUpdated", details); + }); + chrome.webNavigation.onErrorOccurred.addListener( + function(details) { + captureEvent("onErrorOccurred", details); + }); + chrome.webNavigation.onTabReplaced.addListener( + function(details) { + captureEvent("onTabReplaced", details); + }); + chrome.webNavigation.onHistoryStateUpdated.addListener( + function(details) { + captureEvent("onHistoryStateUpdated", details); + }); +} + +// Returns the usual order of navigation events. +function navigationOrder(prefix) { + return [ prefix + "onBeforeNavigate", + prefix + "onCommitted", + prefix + "onDOMContentLoaded", + prefix + "onCompleted" ]; +} + +// Returns the constraints expressing that a frame is an iframe of another +// frame. +function isIFrameOf(iframe, main_frame) { + return [ main_frame + "onCommitted", + iframe + "onBeforeNavigate", + iframe + "onCompleted", + main_frame + "onCompleted" ]; +} + +// Returns the constraint expressing that a frame was loaded by another. +function isLoadedBy(target, source) { + return [ source + "onDOMContentLoaded", target + "onBeforeNavigate"]; +} diff --git a/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/main.html b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/main.html new file mode 100644 index 0000000..140fab7 --- /dev/null +++ b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/main.html @@ -0,0 +1 @@ +<script src=main.js></script> diff --git a/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/main.js b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/main.js new file mode 100644 index 0000000..4f5be77 --- /dev/null +++ b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/main.js @@ -0,0 +1,19 @@ +// Copyright 2015 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. + +onload = function() { + var port = location.search.slice(1); + setTimeout(function() { + var f = document.createElement('iframe'); + window.onmessage = function(event) { + chrome.test.assertEq(f.contentWindow, event.source); + chrome.test.assertEq('http://a.com:' + port, event.origin); + chrome.test.assertEq('a.com: go to b.com', event.data); + f.src = f.src.replace('a.com', 'b.com'); + }; + f.src = 'http://a.com:' + port + + '/extensions/api_test/webnavigation/crossProcessIframe/frame.html'; + document.body.appendChild(f); + }, 0); +}; diff --git a/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/manifest.json b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/manifest.json new file mode 100644 index 0000000..81a3ff1 --- /dev/null +++ b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/manifest.json @@ -0,0 +1,10 @@ +{ + "name": "webNavigation", + "version": "1.0", + "manifest_version": 2, + "description": "Tests the webNavigation API events - crossProcessIframe.", + "background": { + "page": "test_crossProcessIframe.html" + }, + "permissions": ["webNavigation", "tabs"] +} diff --git a/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/test_crossProcessIframe.html b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/test_crossProcessIframe.html new file mode 100644 index 0000000..f2d0a82 --- /dev/null +++ b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/test_crossProcessIframe.html @@ -0,0 +1,2 @@ +<script src="test_crossProcessIframe.js"></script> +<script src="framework.js"></script> diff --git a/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/test_crossProcessIframe.js b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/test_crossProcessIframe.js new file mode 100644 index 0000000..8912831 --- /dev/null +++ b/chrome/test/data/extensions/api_test/webnavigation/crossProcessIframe/test_crossProcessIframe.js @@ -0,0 +1,216 @@ +// Copyright 2015 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. + +onload = function() { + debug = true; + var getURL = chrome.extension.getURL; + var URL_MAIN = getURL('main.html'); + var URL_FRAME1 = 'http://a.com:PORT/extensions/api_test/webnavigation/' + + 'crossProcessIframe/frame.html'; + var URL_FRAME2 = 'http://b.com:PORT/extensions/api_test/webnavigation/' + + 'crossProcessIframe/frame.html'; + var URL_FRAME3 = 'http://c.com:PORT/extensions/api_test/webnavigation/' + + 'crossProcessIframe/frame.html'; + chrome.tabs.create({'url': 'about:blank'}, function(tab) { + var tabId = tab.id; + chrome.test.getConfig(function(config) { + var fixPort = function(url) { + return url.replace(/PORT/g, config.testServer.port); + }; + URL_FRAME1 = fixPort(URL_FRAME1); + URL_FRAME2 = fixPort(URL_FRAME2); + URL_FRAME3 = fixPort(URL_FRAME3); + + chrome.test.runTests([ + // Navigates from an extension page to a HTTP page which causes a + // process switch. The extension page embeds a same-process iframe which + // embeds another frame that navigates three times (cross-process): + // c. Loaded by the parent frame. + // d. Navigated by the parent frame. + // e. Navigated by the child frame. + // Tests whether the frameId stays constant across navigations. + function crossProcessIframe() { + expect([ + { label: 'main-onBeforeNavigate', + event: 'onBeforeNavigate', + details: { frameId: 0, + parentFrameId: -1, + processId: 0, + tabId: 0, + timeStamp: 0, + url: URL_MAIN }}, + { label: 'main-onCommitted', + event: 'onCommitted', + details: { frameId: 0, + processId: 0, + tabId: 0, + timeStamp: 0, + transitionQualifiers: [], + transitionType: 'link', + url: URL_MAIN }}, + { label: 'main-onDOMContentLoaded', + event: 'onDOMContentLoaded', + details: { frameId: 0, + processId: 0, + tabId: 0, + timeStamp: 0, + url: URL_MAIN }}, + { label: 'main-onCompleted', + event: 'onCompleted', + details: { frameId: 0, + processId: 0, + tabId: 0, + timeStamp: 0, + url: URL_MAIN }}, + // pre-a.com is the navigation before the process swap. + { label: 'pre-a.com-onBeforeNavigate', + event: 'onBeforeNavigate', + details: { frameId: 1, + parentFrameId: 0, + processId: 0, + tabId: 0, + timeStamp: 0, + url: URL_FRAME1 }}, + { label: 'pre-a.com-onErrorOccurred', + event: 'onErrorOccurred', + details: { error: 'net::ERR_ABORTED', + frameId: 1, + processId: 0, + tabId: 0, + timeStamp: 0, + url: URL_FRAME1 }}, + { label: 'a.com-onBeforeNavigate', + event: 'onBeforeNavigate', + details: { frameId: 1, + parentFrameId: 0, + processId: 1, + tabId: 0, + timeStamp: 0, + url: URL_FRAME1 }}, + { label: 'a.com-onCommitted', + event: 'onCommitted', + details: { frameId: 1, + processId: 1, + tabId: 0, + timeStamp: 0, + transitionQualifiers: [], + transitionType: 'auto_subframe', + url: URL_FRAME1 }}, + { label: 'a.com-onDOMContentLoaded', + event: 'onDOMContentLoaded', + details: { frameId: 1, + processId: 1, + tabId: 0, + timeStamp: 0, + url: URL_FRAME1 }}, + { label: 'a.com-onCompleted', + event: 'onCompleted', + details: { frameId: 1, + processId: 1, + tabId: 0, + timeStamp: 0, + url: URL_FRAME1 }}, + // There is no onBeforeNavigate and onErrorOccurred (like the other + // navigations in this test) because this navigation is triggered by + // a frame in a different process, so the navigation directly goes + // through the browser. This difference will be resolved once + // PlzNavigate goes live. + { label: 'b.com-onBeforeNavigate', + event: 'onBeforeNavigate', + details: { frameId: 1, + parentFrameId: 0, + processId: 2, + tabId: 0, + timeStamp: 0, + url: URL_FRAME2 }}, + { label: 'b.com-onCommitted', + event: 'onCommitted', + details: { frameId: 1, + processId: 2, + tabId: 0, + timeStamp: 0, + transitionQualifiers: [], + transitionType: 'manual_subframe', + url: URL_FRAME2 }}, + { label: 'b.com-onDOMContentLoaded', + event: 'onDOMContentLoaded', + details: { frameId: 1, + processId: 2, + tabId: 0, + timeStamp: 0, + url: URL_FRAME2 }}, + { label: 'b.com-onCompleted', + event: 'onCompleted', + details: { frameId: 1, + processId: 2, + tabId: 0, + timeStamp: 0, + url: URL_FRAME2 }}, + // pre-c.com is the navigation before the process swap. + { label: 'pre-c.com-onBeforeNavigate', + event: 'onBeforeNavigate', + details: { frameId: 1, + parentFrameId: 0, + processId: 2, + tabId: 0, + timeStamp: 0, + url: URL_FRAME3 }}, + { label: 'pre-c.com-onErrorOccurred', + event: 'onErrorOccurred', + details: { error: 'net::ERR_ABORTED', + frameId: 1, + processId: 2, + tabId: 0, + timeStamp: 0, + url: URL_FRAME3 }}, + { label: 'c.com-onBeforeNavigate', + event: 'onBeforeNavigate', + details: { frameId: 1, + parentFrameId: 0, + processId: 3, + tabId: 0, + timeStamp: 0, + url: URL_FRAME3 }}, + { label: 'c.com-onCommitted', + event: 'onCommitted', + details: { frameId: 1, + processId: 3, + tabId: 0, + timeStamp: 0, + transitionQualifiers: [], + transitionType: 'manual_subframe', + url: URL_FRAME3 }}, + { label: 'c.com-onDOMContentLoaded', + event: 'onDOMContentLoaded', + details: { frameId: 1, + processId: 3, + tabId: 0, + timeStamp: 0, + url: URL_FRAME3 }}, + { label: 'c.com-onCompleted', + event: 'onCompleted', + details: { frameId: 1, + processId: 3, + tabId: 0, + timeStamp: 0, + url: URL_FRAME3 }}], + [ + navigationOrder('main-'), + navigationOrder('a.com-'), + navigationOrder('b.com-'), + navigationOrder('c.com-'), + ['pre-a.com-onBeforeNavigate', 'a.com-onBeforeNavigate', + 'pre-a.com-onErrorOccurred'], + ['pre-c.com-onBeforeNavigate', 'c.com-onBeforeNavigate', + 'pre-c.com-onErrorOccurred']]); + + chrome.tabs.update(tabId, { + url: URL_MAIN + '?' + config.testServer.port + }); + }, + + ]); + }); + }); +}; diff --git a/chrome/test/data/extensions/api_test/webrequest/framework.js b/chrome/test/data/extensions/api_test/webrequest/framework.js index 7faa458..c9b7c15 100644 --- a/chrome/test/data/extensions/api_test/webrequest/framework.js +++ b/chrome/test/data/extensions/api_test/webrequest/framework.js @@ -191,11 +191,6 @@ function isUnexpectedDetachedRequest(name, details) { } function captureEvent(name, details, callback) { - // frameId should be -1 or positive, but is sometimes -2 (MSG_ROUTING_NONE). - // TODO(robwu): This will be resolved once crbug.com/432875 is resolved. - if (details.frameId === -2) - details.frameId = -1; - // Ignore system-level requests like safebrowsing updates and favicon fetches // since they are unpredictable. if (details.type == "other" || diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index 1834494..e693c96 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc @@ -318,6 +318,10 @@ RenderFrameHost* RenderFrameHostImpl::GetParent() { return parent_node->current_frame_host(); } +int RenderFrameHostImpl::GetFrameTreeNodeId() { + return frame_tree_node_->frame_tree_node_id(); +} + const std::string& RenderFrameHostImpl::GetFrameName() { return frame_tree_node_->frame_name(); } diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h index 07f78db..b457a07 100644 --- a/content/browser/frame_host/render_frame_host_impl.h +++ b/content/browser/frame_host/render_frame_host_impl.h @@ -140,6 +140,7 @@ class CONTENT_EXPORT RenderFrameHostImpl SiteInstanceImpl* GetSiteInstance() override; RenderProcessHost* GetProcess() override; RenderFrameHost* GetParent() override; + int GetFrameTreeNodeId() override; const std::string& GetFrameName() override; bool IsCrossProcessSubframe() override; GURL GetLastCommittedURL() override; diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc index 67eaf08a..92f06c3 100644 --- a/content/browser/loader/resource_dispatcher_host_impl.cc +++ b/content/browser/loader/resource_dispatcher_host_impl.cc @@ -1182,8 +1182,7 @@ void ResourceDispatcherHostImpl::UpdateRequestForTransfer( // ResourceRequestInfo rather than caching it locally. This lets us update // the info object when a transfer occurs. info->UpdateForTransfer(child_id, route_id, request_data.origin_pid, - request_id, request_data.parent_render_frame_id, - filter_->GetWeakPtr()); + request_id, filter_->GetWeakPtr()); // Update maps that used the old IDs, if necessary. Some transfers in tests // do not actually use a different ID, so not all maps need to be updated. @@ -1414,7 +1413,6 @@ void ResourceDispatcherHostImpl::BeginRequest( request_data.render_frame_id, request_data.is_main_frame, request_data.parent_is_main_frame, - request_data.parent_render_frame_id, request_data.resource_type, request_data.transition_type, request_data.should_replace_current_entry, @@ -1712,7 +1710,6 @@ ResourceRequestInfoImpl* ResourceDispatcherHostImpl::CreateRequestInfo( render_frame_route_id, false, // is_main_frame false, // parent_is_main_frame - -1, // parent_render_frame_id RESOURCE_TYPE_SUB_RESOURCE, ui::PAGE_TRANSITION_LINK, false, // should_replace_current_entry @@ -2152,7 +2149,6 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest( request_id_, -1, // request_data.render_frame_id, info.is_main_frame, info.parent_is_main_frame, - -1, // request_data.parent_render_frame_id, resource_type, info.common_params.transition, // should_replace_current_entry. This was only maintained at layer for // request transfers and isn't needed for browser-side navigations. diff --git a/content/browser/loader/resource_dispatcher_host_unittest.cc b/content/browser/loader/resource_dispatcher_host_unittest.cc index f5a97aa..7fbe399 100644 --- a/content/browser/loader/resource_dispatcher_host_unittest.cc +++ b/content/browser/loader/resource_dispatcher_host_unittest.cc @@ -153,7 +153,6 @@ static ResourceHostMsg_Request CreateResourceRequest(const char* method, request.should_reset_appcache = false; request.is_main_frame = true; request.parent_is_main_frame = false; - request.parent_render_frame_id = -1; request.transition_type = ui::PAGE_TRANSITION_LINK; request.allow_download = true; return request; diff --git a/content/browser/loader/resource_request_info_impl.cc b/content/browser/loader/resource_request_info_impl.cc index a269baf..0f61829 100644 --- a/content/browser/loader/resource_request_info_impl.cc +++ b/content/browser/loader/resource_request_info_impl.cc @@ -71,7 +71,6 @@ void ResourceRequestInfo::AllocateForTesting(net::URLRequest* request, render_frame_id, // render_frame_id is_main_frame, // is_main_frame parent_is_main_frame, // parent_is_main_frame - 0, // parent_render_frame_id resource_type, // resource_type ui::PAGE_TRANSITION_LINK, // transition_type false, // should_replace_current_entry @@ -132,7 +131,6 @@ ResourceRequestInfoImpl::ResourceRequestInfoImpl( int render_frame_id, bool is_main_frame, bool parent_is_main_frame, - int parent_render_frame_id, ResourceType resource_type, ui::PageTransition transition_type, bool should_replace_current_entry, @@ -162,7 +160,6 @@ ResourceRequestInfoImpl::ResourceRequestInfoImpl( render_frame_id_(render_frame_id), is_main_frame_(is_main_frame), parent_is_main_frame_(parent_is_main_frame), - parent_render_frame_id_(parent_render_frame_id), should_replace_current_entry_(should_replace_current_entry), is_download_(is_download), is_stream_(is_stream), @@ -240,10 +237,6 @@ bool ResourceRequestInfoImpl::ParentIsMainFrame() const { return parent_is_main_frame_; } -int ResourceRequestInfoImpl::GetParentRenderFrameID() const { - return parent_render_frame_id_; -} - ResourceType ResourceRequestInfoImpl::GetResourceType() const { return resource_type_; } @@ -330,13 +323,11 @@ void ResourceRequestInfoImpl::UpdateForTransfer( int route_id, int origin_pid, int request_id, - int parent_render_frame_id, base::WeakPtr<ResourceMessageFilter> filter) { child_id_ = child_id; route_id_ = route_id; origin_pid_ = origin_pid; request_id_ = request_id; - parent_render_frame_id_ = parent_render_frame_id; filter_ = filter; } diff --git a/content/browser/loader/resource_request_info_impl.h b/content/browser/loader/resource_request_info_impl.h index bb6c948..3fbce96 100644 --- a/content/browser/loader/resource_request_info_impl.h +++ b/content/browser/loader/resource_request_info_impl.h @@ -49,7 +49,6 @@ class ResourceRequestInfoImpl : public ResourceRequestInfo, int render_frame_id, bool is_main_frame, bool parent_is_main_frame, - int parent_render_frame_id, ResourceType resource_type, ui::PageTransition transition_type, bool should_replace_current_entry, @@ -79,7 +78,6 @@ class ResourceRequestInfoImpl : public ResourceRequestInfo, int GetRenderFrameID() const override; bool IsMainFrame() const override; bool ParentIsMainFrame() const override; - int GetParentRenderFrameID() const override; ResourceType GetResourceType() const override; int GetProcessType() const override; blink::WebReferrerPolicy GetReferrerPolicy() const override; @@ -118,7 +116,6 @@ class ResourceRequestInfoImpl : public ResourceRequestInfo, int route_id, int origin_pid, int request_id, - int parent_render_frame_id, base::WeakPtr<ResourceMessageFilter> filter); // CrossSiteResourceHandler for this request. May be null. @@ -204,7 +201,6 @@ class ResourceRequestInfoImpl : public ResourceRequestInfo, int render_frame_id_; bool is_main_frame_; bool parent_is_main_frame_; - int parent_render_frame_id_; bool should_replace_current_entry_; bool is_download_; bool is_stream_; diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 1ea9beb..150396e 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -773,6 +773,12 @@ RenderFrameHostImpl* WebContentsImpl::GetFocusedFrame() { return focused_node->current_frame_host(); } +RenderFrameHostImpl* WebContentsImpl::FindFrameByFrameTreeNodeId( + int frame_tree_node_id) { + FrameTreeNode* frame = frame_tree_.FindByID(frame_tree_node_id); + return frame ? frame->current_frame_host() : nullptr; +} + void WebContentsImpl::ForEachFrame( const base::Callback<void(RenderFrameHost*)>& on_frame) { frame_tree_.ForEach(base::Bind(&ForEachFrameInternal, on_frame)); diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h index 6c35bb7..a247d03 100644 --- a/content/browser/web_contents/web_contents_impl.h +++ b/content/browser/web_contents/web_contents_impl.h @@ -240,6 +240,8 @@ class CONTENT_EXPORT WebContentsImpl RenderProcessHost* GetRenderProcessHost() const override; RenderFrameHostImpl* GetMainFrame() override; RenderFrameHostImpl* GetFocusedFrame() override; + RenderFrameHostImpl* FindFrameByFrameTreeNodeId( + int frame_tree_node_id) override; void ForEachFrame( const base::Callback<void(RenderFrameHost*)>& on_frame) override; void SendToAllFrames(IPC::Message* message) override; diff --git a/content/public/browser/render_frame_host.h b/content/public/browser/render_frame_host.h index a5c0424..4a45df2 100644 --- a/content/public/browser/render_frame_host.h +++ b/content/public/browser/render_frame_host.h @@ -37,6 +37,16 @@ class CONTENT_EXPORT RenderFrameHost : public IPC::Listener, // Returns nullptr if the IDs do not correspond to a live RenderFrameHost. static RenderFrameHost* FromID(int render_process_id, int render_frame_id); + // Returns the current RenderFrameHost associated with the frame identified by + // the given FrameTreeNode ID, in any WebContents. The frame may change its + // current RenderFrameHost over time, so the returned RenderFrameHost can be + // different from the RenderFrameHost that returned the ID via + // GetFrameTreeNodeId(). See GetFrameTreeNodeId for more details. + // Use WebContents::FindFrameByFrameTreeNodeId to find a RenderFrameHost in + // a specific WebContents. + // Returns nullptr if the frame does not exist. + static RenderFrameHost* FromFrameTreeNodeId(int frame_tree_node_id); + #if defined(OS_ANDROID) // Globally allows for injecting JavaScript into the main world. This feature // is present only to support Android WebView and must not be used in other @@ -68,6 +78,17 @@ class CONTENT_EXPORT RenderFrameHost : public IPC::Listener, // current RenderFrameHost. virtual RenderFrameHost* GetParent() = 0; + // Returns the FrameTreeNode ID for this frame. This ID is browser-global and + // uniquely identifies a frame that hosts content. The identifier is fixed at + // the creation of the frame and stays constant for the lifetime of the frame. + // When the frame is removed, the ID is not used again. + // + // A RenderFrameHost is tied to a process. Due to cross-process navigations, + // the RenderFrameHost may have a shorter lifetime than a frame. Consequently, + // the same FrameTreeNode ID may refer to a different RenderFrameHost after a + // navigation. + virtual int GetFrameTreeNodeId() = 0; + // Returns the assigned name of the frame, the name of the iframe tag // declaring it. For example, <iframe name="framename">[...]</iframe>. It is // quite possible for a frame to have no name, in which case GetFrameName will diff --git a/content/public/browser/resource_request_info.h b/content/public/browser/resource_request_info.h index e65bce0..d5ee296 100644 --- a/content/public/browser/resource_request_info.h +++ b/content/public/browser/resource_request_info.h @@ -94,13 +94,9 @@ class ResourceRequestInfo { // True if GetRenderFrameID() represents a main frame in the RenderView. virtual bool IsMainFrame() const = 0; - // True if GetParentRenderFrameID() represents a main frame in the RenderView. + // True if the frame's parent represents a main frame in the RenderView. virtual bool ParentIsMainFrame() const = 0; - // Routing ID of parent frame of frame that sent this resource request. - // -1 if unknown / invalid. - virtual int GetParentRenderFrameID() const = 0; - // Returns the associated resource type. virtual ResourceType GetResourceType() const = 0; diff --git a/content/public/browser/web_contents.h b/content/public/browser/web_contents.h index 6f68c96..bc13dc9 100644 --- a/content/public/browser/web_contents.h +++ b/content/public/browser/web_contents.h @@ -214,6 +214,12 @@ class WebContents : public PageNavigator, // Returns the focused frame for the currently active view. virtual RenderFrameHost* GetFocusedFrame() = 0; + // Returns the current RenderFrameHost for a given FrameTreeNode ID if it is + // part of this tab. See RenderFrameHost::GetFrameTreeNodeId for documentation + // on this ID. + virtual RenderFrameHost* FindFrameByFrameTreeNodeId( + int frame_tree_node_id) = 0; + // Calls |on_frame| for each frame in the currently active view. // Note: The RenderFrameHost parameter is not guaranteed to have a live // RenderFrame counterpart in the renderer process. Callbacks should check diff --git a/extensions/browser/api/web_request/web_request_api.cc b/extensions/browser/api/web_request/web_request_api.cc index 871e3e0..b3855a3 100644 --- a/extensions/browser/api/web_request/web_request_api.cc +++ b/extensions/browser/api/web_request/web_request_api.cc @@ -40,6 +40,7 @@ #include "extensions/browser/api/web_request/web_request_event_router_delegate.h" #include "extensions/browser/api/web_request/web_request_time_tracker.h" #include "extensions/browser/event_router.h" +#include "extensions/browser/extension_api_frame_id_map.h" #include "extensions/browser/extension_prefs.h" #include "extensions/browser/extension_registry.h" #include "extensions/browser/extension_system.h" @@ -131,10 +132,6 @@ const char* GetRequestStageAsString( return "Not reached"; } -int GetFrameId(bool is_main_frame, int frame_id) { - return is_main_frame ? 0 : frame_id; -} - bool IsWebRequestEvent(const std::string& event_name) { std::string web_request_event_name(event_name); if (base::StartsWith(web_request_event_name, @@ -205,9 +202,7 @@ bool GetWebViewInfo(const net::URLRequest* request, void ExtractRequestInfoDetails(const net::URLRequest* request, bool* is_main_frame, - int* frame_id, - bool* parent_is_main_frame, - int* parent_frame_id, + int* render_frame_id, int* render_process_host_id, int* routing_id, ResourceType* resource_type) { @@ -215,10 +210,8 @@ void ExtractRequestInfoDetails(const net::URLRequest* request, if (!info) return; - *frame_id = info->GetRenderFrameID(); + *render_frame_id = info->GetRenderFrameID(); *is_main_frame = info->IsMainFrame(); - *parent_frame_id = info->GetParentRenderFrameID(); - *parent_is_main_frame = info->ParentIsMainFrame(); *render_process_host_id = info->GetChildID(); *routing_id = info->GetRouteID(); @@ -229,6 +222,21 @@ void ExtractRequestInfoDetails(const net::URLRequest* request, *resource_type = content::RESOURCE_TYPE_LAST_TYPE; } +// Extracts a pair of IDs to identify the RenderFrameHost. These IDs are used to +// get the frame ID and parent frame ID from ExtensionApiFrameIdMap, and then +// stored in |dict| by DispatchEventToListeners or SendOnMessageEventOnUI. +void ExtractRenderFrameInfo(base::DictionaryValue* dict, + int* render_process_id, + int* render_frame_id) { + if (!dict->GetInteger(keys::kFrameIdKey, render_frame_id) || + !dict->GetInteger(keys::kProcessIdKey, render_process_id)) { + *render_process_id = -1; + *render_frame_id = -1; + } + // kFrameIdKey will be overwritten later, so it's not removed here. + dict->Remove(keys::kProcessIdKey, nullptr); +} + // Extracts the body from |request| and writes the data into |out|. void ExtractRequestInfoBody(const net::URLRequest* request, base::DictionaryValue* out) { @@ -361,6 +369,16 @@ void SendOnMessageEventOnUI( return; scoped_ptr<base::ListValue> event_args(new base::ListValue); + int render_process_host_id = -1; + int render_frame_id = -1; + ExtractRenderFrameInfo(event_argument.get(), &render_process_host_id, + &render_frame_id); + content::RenderFrameHost* rfh = + content::RenderFrameHost::FromID(render_process_host_id, render_frame_id); + event_argument->SetInteger(keys::kFrameIdKey, + ExtensionApiFrameIdMap::GetFrameId(rfh)); + event_argument->SetInteger(keys::kParentFrameIdKey, + ExtensionApiFrameIdMap::GetParentFrameId(rfh)); event_args->Append(event_argument.release()); EventRouter* event_router = EventRouter::Get(browser_context); @@ -527,8 +545,8 @@ struct ExtensionWebRequestEventRouter::EventListener { // is also used to find and remove an event listener when an extension is // unloaded. At this point, the event listener cannot be mapped back to // the original process, so 0 is used instead of the actual process ID. - DCHECK(embedder_process_id == 0 || that.embedder_process_id == 0); - return false; + if (embedder_process_id == 0 || that.embedder_process_id == 0) + return false; } if (embedder_process_id != that.embedder_process_id) @@ -744,28 +762,24 @@ void ExtensionWebRequestEventRouter::ExtractRequestInfo( const net::URLRequest* request, base::DictionaryValue* out) { bool is_main_frame = false; - int frame_id = -1; - bool parent_is_main_frame = false; - int parent_frame_id = -1; - int frame_id_for_extension = -1; - int parent_frame_id_for_extension = -1; + int render_frame_id = -1; int render_process_host_id = -1; int routing_id = -1; ResourceType resource_type = content::RESOURCE_TYPE_LAST_TYPE; - ExtractRequestInfoDetails(request, &is_main_frame, &frame_id, - &parent_is_main_frame, &parent_frame_id, + ExtractRequestInfoDetails(request, &is_main_frame, &render_frame_id, &render_process_host_id, &routing_id, &resource_type); - frame_id_for_extension = GetFrameId(is_main_frame, frame_id); - parent_frame_id_for_extension = GetFrameId(parent_is_main_frame, - parent_frame_id); out->SetString(keys::kRequestIdKey, base::Uint64ToString(request->identifier())); out->SetString(keys::kUrlKey, request->url().spec()); out->SetString(keys::kMethodKey, request->method()); - out->SetInteger(keys::kFrameIdKey, frame_id_for_extension); - out->SetInteger(keys::kParentFrameIdKey, parent_frame_id_for_extension); + // Note: This (frameId, processId) pair is removed by ExtractRenderFrameInfo, + // and finally restored in DispatchEventToListeners or SendOnMessageEventOnUI. + // TODO(robwu): This is ugly. Create a proper data structure to separate these + // two IDs from the dictionary, so that kFrameIdKey has only one meaning. + out->SetInteger(keys::kFrameIdKey, render_frame_id); + out->SetInteger(keys::kProcessIdKey, render_process_host_id); out->SetString(keys::kTypeKey, helpers::ResourceTypeToString(resource_type)); out->SetDouble(keys::kTimeStampKey, base::Time::Now().ToDoubleT() * 1000); if (web_request_event_router_delegate_) { @@ -1244,21 +1258,12 @@ bool ExtensionWebRequestEventRouter::DispatchEvent( // TODO(mpcomplete): Consider consolidating common (extension_id,json_args) // pairs into a single message sent to a list of sub_event_names. int num_handlers_blocking = 0; - for (const EventListener* listener : listeners) { - // Filter out the optional keys that this listener didn't request. - scoped_ptr<base::ListValue> args_filtered(args.DeepCopy()); - base::DictionaryValue* dict = NULL; - CHECK(args_filtered->GetDictionary(0, &dict) && dict); - if (!(listener->extra_info_spec & ExtraInfoSpec::REQUEST_HEADERS)) - dict->Remove(keys::kRequestHeadersKey, NULL); - if (!(listener->extra_info_spec & ExtraInfoSpec::RESPONSE_HEADERS)) - dict->Remove(keys::kResponseHeadersKey, NULL); - EventRouter::DispatchEventToSender( - listener->ipc_sender.get(), browser_context, listener->extension_id, - listener->histogram_value, listener->sub_event_name, - std::move(args_filtered), EventRouter::USER_GESTURE_UNKNOWN, - EventFilteringInfo()); + scoped_ptr<std::vector<EventListener>> listeners_to_dispatch( + new std::vector<EventListener>()); + listeners_to_dispatch->reserve(listeners.size()); + for (const EventListener* listener : listeners) { + listeners_to_dispatch->push_back(*listener); if (listener->extra_info_spec & (ExtraInfoSpec::BLOCKING | ExtraInfoSpec::ASYNC_BLOCKING)) { listener->blocked_requests.insert(request->identifier()); @@ -1276,6 +1281,22 @@ bool ExtensionWebRequestEventRouter::DispatchEvent( } } + // TODO(robwu): Avoid unnecessary copy, by changing |args| to be a + // scoped_ptr<base::DictionaryValue> and transferring the ownership. + const base::DictionaryValue* dict = nullptr; + CHECK(args.GetDictionary(0, &dict) && dict); + base::DictionaryValue* args_copy = dict->DeepCopy(); + + int render_process_host_id = -1; + int render_frame_id = -1; + ExtractRenderFrameInfo(args_copy, &render_process_host_id, &render_frame_id); + + ExtensionApiFrameIdMap::Get()->GetFrameIdOnIO( + render_process_host_id, render_frame_id, + base::Bind(&ExtensionWebRequestEventRouter::DispatchEventToListeners, + AsWeakPtr(), browser_context, + base::Passed(&listeners_to_dispatch), base::Owned(args_copy))); + if (num_handlers_blocking > 0) { BlockedRequest& blocked_request = blocked_requests_[request->identifier()]; blocked_request.request = request; @@ -1288,6 +1309,59 @@ bool ExtensionWebRequestEventRouter::DispatchEvent( return false; } +void ExtensionWebRequestEventRouter::DispatchEventToListeners( + void* browser_context, + scoped_ptr<std::vector<EventListener>> listeners, + base::DictionaryValue* dict, + int extension_api_frame_id, + int extension_api_parent_frame_id) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + DCHECK(listeners.get()); + DCHECK_GT(listeners->size(), 0UL); + DCHECK(dict); + + dict->SetInteger(keys::kFrameIdKey, extension_api_frame_id); + dict->SetInteger(keys::kParentFrameIdKey, extension_api_parent_frame_id); + + std::string event_name = + EventRouter::GetBaseEventName((*listeners)[0].sub_event_name); + DCHECK(IsWebRequestEvent(event_name)); + + const std::set<EventListener>& event_listeners = + listeners_[browser_context][event_name]; + void* cross_browser_context = GetCrossBrowserContext(browser_context); + const std::set<EventListener>* cross_event_listeners = + cross_browser_context ? &listeners_[cross_browser_context][event_name] + : nullptr; + + for (const EventListener& target : *listeners) { + std::set<EventListener>::const_iterator listener = + event_listeners.find(target); + // Ignore listener if it was removed between the thread hops. + if (listener == event_listeners.end()) { + if (!cross_event_listeners) + continue; + listener = cross_event_listeners->find(target); + if (listener == cross_event_listeners->end()) + continue; + } + + // Filter out the optional keys that this listener didn't request. + scoped_ptr<base::ListValue> args_filtered(new base::ListValue); + args_filtered->Append(dict->DeepCopy()); + if (!(listener->extra_info_spec & ExtraInfoSpec::REQUEST_HEADERS)) + dict->Remove(keys::kRequestHeadersKey, nullptr); + if (!(listener->extra_info_spec & ExtraInfoSpec::RESPONSE_HEADERS)) + dict->Remove(keys::kResponseHeadersKey, nullptr); + + EventRouter::DispatchEventToSender( + listener->ipc_sender.get(), browser_context, listener->extension_id, + listener->histogram_value, listener->sub_event_name, + std::move(args_filtered), EventRouter::USER_GESTURE_UNKNOWN, + EventFilteringInfo()); + } +} + void ExtensionWebRequestEventRouter::OnEventHandled( void* browser_context, const std::string& extension_id, @@ -1441,17 +1515,14 @@ void ExtensionWebRequestEventRouter::AddCallbackForPageLoad( bool ExtensionWebRequestEventRouter::IsPageLoad( const net::URLRequest* request) const { bool is_main_frame = false; - int frame_id = -1; - bool parent_is_main_frame = false; - int parent_frame_id = -1; + int render_frame_id = -1; int render_process_host_id = -1; int routing_id = -1; ResourceType resource_type = content::RESOURCE_TYPE_LAST_TYPE; - ExtractRequestInfoDetails(request, &is_main_frame, &frame_id, - &parent_is_main_frame, &parent_frame_id, - &render_process_host_id, - &routing_id, &resource_type); + ExtractRequestInfoDetails(request, &is_main_frame, &render_frame_id, + &render_process_host_id, &routing_id, + &resource_type); return resource_type == content::RESOURCE_TYPE_MAIN_FRAME; } @@ -1591,18 +1662,15 @@ ExtensionWebRequestEventRouter::GetMatchingListeners( *extra_info_spec = 0; bool is_main_frame = false; - int frame_id = -1; - bool parent_is_main_frame = false; - int parent_frame_id = -1; + int render_frame_id = -1; int render_process_host_id = -1; int routing_id = -1; ResourceType resource_type = content::RESOURCE_TYPE_LAST_TYPE; const GURL& url = request->url(); - ExtractRequestInfoDetails(request, &is_main_frame, &frame_id, - &parent_is_main_frame, &parent_frame_id, - &render_process_host_id, - &routing_id, &resource_type); + ExtractRequestInfoDetails(request, &is_main_frame, &render_frame_id, + &render_process_host_id, &routing_id, + &resource_type); bool is_request_from_extension = IsRequestFromExtension(request, extension_info_map); diff --git a/extensions/browser/api/web_request/web_request_api.h b/extensions/browser/api/web_request/web_request_api.h index 4b214ca..191005d 100644 --- a/extensions/browser/api/web_request/web_request_api.h +++ b/extensions/browser/api/web_request/web_request_api.h @@ -342,6 +342,13 @@ class ExtensionWebRequestEventRouter const std::vector<const EventListener*>& listeners, const base::ListValue& args); + void DispatchEventToListeners( + void* browser_context, + scoped_ptr<std::vector<EventListener>> listeners, + base::DictionaryValue* dict, + int extension_api_frame_id, + int extension_api_parent_frame_id); + // Returns a list of event listeners that care about the given event, based // on their filter parameters. |extra_info_spec| will contain the combined // set of extra_info_spec flags that every matching listener asked for. diff --git a/extensions/browser/api/web_request/web_request_api_constants.cc b/extensions/browser/api/web_request/web_request_api_constants.cc index 4bc0c0e..fd10ca1 100644 --- a/extensions/browser/api/web_request/web_request_api_constants.cc +++ b/extensions/browser/api/web_request/web_request_api_constants.cc @@ -10,6 +10,7 @@ const char kChallengerKey[] = "challenger"; const char kErrorKey[] = "error"; const char kFrameIdKey[] = "frameId"; const char kParentFrameIdKey[] = "parentFrameId"; +const char kProcessIdKey[] = "processId"; const char kFromCache[] = "fromCache"; const char kHostKey[] = "host"; const char kIpKey[] = "ip"; diff --git a/extensions/browser/api/web_request/web_request_api_constants.h b/extensions/browser/api/web_request/web_request_api_constants.h index 57bf535..ce51d06 100644 --- a/extensions/browser/api/web_request/web_request_api_constants.h +++ b/extensions/browser/api/web_request/web_request_api_constants.h @@ -14,6 +14,7 @@ extern const char kChallengerKey[]; extern const char kErrorKey[]; extern const char kFrameIdKey[]; extern const char kParentFrameIdKey[]; +extern const char kProcessIdKey[]; extern const char kFromCache[]; extern const char kHostKey[]; extern const char kIpKey[]; diff --git a/extensions/browser/extension_api_frame_id_map.cc b/extensions/browser/extension_api_frame_id_map.cc new file mode 100644 index 0000000..72949be --- /dev/null +++ b/extensions/browser/extension_api_frame_id_map.cc @@ -0,0 +1,245 @@ +// Copyright 2015 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 "extensions/browser/extension_api_frame_id_map.h" + +#include <tuple> + +#include "content/public/browser/browser_thread.h" +#include "content/public/browser/render_frame_host.h" +#include "content/public/browser/render_process_host.h" +#include "content/public/browser/web_contents.h" +#include "content/public/common/child_process_host.h" + +namespace extensions { + +namespace { + +// The map is accessed on the IO and UI thread, so construct it once and never +// delete it. +base::LazyInstance<ExtensionApiFrameIdMap>::Leaky g_map_instance = + LAZY_INSTANCE_INITIALIZER; + +} // namespace + +const int ExtensionApiFrameIdMap::kInvalidFrameId = -1; + +ExtensionApiFrameIdMap::CachedFrameIdPair::CachedFrameIdPair() + : frame_id(kInvalidFrameId), parent_frame_id(kInvalidFrameId) {} + +ExtensionApiFrameIdMap::CachedFrameIdPair::CachedFrameIdPair( + int frame_id, + int parent_frame_id) + : frame_id(frame_id), parent_frame_id(parent_frame_id) {} + +ExtensionApiFrameIdMap::RenderFrameIdKey::RenderFrameIdKey() + : render_process_id(content::ChildProcessHost::kInvalidUniqueID), + frame_routing_id(MSG_ROUTING_NONE) {} + +ExtensionApiFrameIdMap::RenderFrameIdKey::RenderFrameIdKey( + int render_process_id, + int frame_routing_id) + : render_process_id(render_process_id), + frame_routing_id(frame_routing_id) {} + +ExtensionApiFrameIdMap::FrameIdCallbacks::FrameIdCallbacks() + : is_iterating(false) {} + +ExtensionApiFrameIdMap::FrameIdCallbacks::~FrameIdCallbacks() {} + +bool ExtensionApiFrameIdMap::RenderFrameIdKey::operator<( + const RenderFrameIdKey& other) const { + return std::tie(render_process_id, frame_routing_id) < + std::tie(other.render_process_id, other.frame_routing_id); +} + +bool ExtensionApiFrameIdMap::RenderFrameIdKey::operator==( + const RenderFrameIdKey& other) const { + return render_process_id == other.render_process_id && + frame_routing_id == other.frame_routing_id; +} + +ExtensionApiFrameIdMap::ExtensionApiFrameIdMap() {} + +ExtensionApiFrameIdMap::~ExtensionApiFrameIdMap() {} + +ExtensionApiFrameIdMap* ExtensionApiFrameIdMap::Get() { + return g_map_instance.Pointer(); +} + +int ExtensionApiFrameIdMap::GetFrameId(content::RenderFrameHost* rfh) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + + if (!rfh) + return kInvalidFrameId; + if (rfh->GetParent()) + return rfh->GetFrameTreeNodeId(); + return 0; // Main frame. +} + +int ExtensionApiFrameIdMap::GetParentFrameId(content::RenderFrameHost* rfh) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + + return rfh ? GetFrameId(rfh->GetParent()) : kInvalidFrameId; +} + +content::RenderFrameHost* ExtensionApiFrameIdMap::GetRenderFrameHostById( + content::WebContents* web_contents, + int frame_id) { + // Although it is technically possible to map |frame_id| to a RenderFrameHost + // without WebContents, we choose to not do that because in the extension API + // frameIds are only guaranteed to be meaningful in combination with a tabId. + if (!web_contents) + return nullptr; + + if (frame_id == kInvalidFrameId) + return nullptr; + + if (frame_id == 0) + return web_contents->GetMainFrame(); + + DCHECK_GE(frame_id, 1); + return web_contents->FindFrameByFrameTreeNodeId(frame_id); +} + +ExtensionApiFrameIdMap::CachedFrameIdPair ExtensionApiFrameIdMap::KeyToValue( + const RenderFrameIdKey& key) const { + content::RenderFrameHost* rfh = content::RenderFrameHost::FromID( + key.render_process_id, key.frame_routing_id); + return CachedFrameIdPair(GetFrameId(rfh), GetParentFrameId(rfh)); +} + +ExtensionApiFrameIdMap::CachedFrameIdPair +ExtensionApiFrameIdMap::LookupFrameIdOnUI(const RenderFrameIdKey& key) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + + FrameIdMap::const_iterator frame_id_iter = frame_id_map_.find(key); + if (frame_id_iter != frame_id_map_.end()) + return frame_id_iter->second; + + CachedFrameIdPair cached_frame_id_pair = KeyToValue(key); + // Don't save invalid values in the map. + if (cached_frame_id_pair.frame_id == kInvalidFrameId) + return cached_frame_id_pair; + + auto kvpair = FrameIdMap::value_type(key, cached_frame_id_pair); + base::AutoLock lock(frame_id_map_lock_); + return frame_id_map_.insert(kvpair).first->second; +} + +void ExtensionApiFrameIdMap::ReceivedFrameIdOnIO( + const RenderFrameIdKey& key, + const CachedFrameIdPair& cached_frame_id_pair) { + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + + FrameIdCallbacksMap::iterator map_iter = callbacks_map_.find(key); + if (map_iter == callbacks_map_.end()) { + // Can happen if ReceivedFrameIdOnIO was called after the frame ID was + // resolved (e.g. via GetFrameIdOnIO), but before PostTaskAndReply replied. + return; + } + + FrameIdCallbacks& callbacks = map_iter->second; + + if (callbacks.is_iterating) + return; + callbacks.is_iterating = true; + + // Note: Extra items can be appended to |callbacks| during this loop if a + // callback calls GetFrameIdOnIO(). + for (std::list<FrameIdCallback>::iterator it = callbacks.callbacks.begin(); + it != callbacks.callbacks.end(); ++it) { + it->Run(cached_frame_id_pair.frame_id, + cached_frame_id_pair.parent_frame_id); + } + callbacks_map_.erase(key); +} + +void ExtensionApiFrameIdMap::GetFrameIdOnIO(int render_process_id, + int frame_routing_id, + const FrameIdCallback& callback) { + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + + if (frame_routing_id <= -1) { + // frame_routing_id == -2 = MSG_ROUTING_NONE -> not a RenderFrameHost. + // frame_routing_id == -1 -> should be MSG_ROUTING_NONE, but there are + // callers that use "-1" for unknown frames. + // TODO(robwu): Enable assertion when all callers have been fixed. + // DCHECK_EQ(MSG_ROUTING_NONE, -1); + callback.Run(kInvalidFrameId, kInvalidFrameId); + return; + } + // A valid routing ID is only meaningful with a valid process ID. + DCHECK_GE(render_process_id, 0); + + const RenderFrameIdKey key(render_process_id, frame_routing_id); + CachedFrameIdPair cached_frame_id_pair; + bool did_find_cached_frame_id_pair = false; + + { + base::AutoLock lock(frame_id_map_lock_); + FrameIdMap::const_iterator frame_id_iter = frame_id_map_.find(key); + if (frame_id_iter != frame_id_map_.end()) { + // This is very likely to happen because CacheFrameId() is called as soon + // as the frame is created. + cached_frame_id_pair = frame_id_iter->second; + did_find_cached_frame_id_pair = true; + } + } + + FrameIdCallbacksMap::iterator map_iter = callbacks_map_.find(key); + + if (did_find_cached_frame_id_pair) { + // Value already cached, thread hopping is not needed. + if (map_iter == callbacks_map_.end()) { + // If the frame ID was cached, then it is likely that there are no pending + // callbacks. So do not unnecessarily copy the callback, but run it. + callback.Run(cached_frame_id_pair.frame_id, + cached_frame_id_pair.parent_frame_id); + } else { + map_iter->second.callbacks.push_back(callback); + ReceivedFrameIdOnIO(key, cached_frame_id_pair); + } + return; + } + + // The key was seen for the first time (or the frame has been removed). + // Hop to the UI thread to look up the extension API frame ID. + callbacks_map_[key].callbacks.push_back(callback); + content::BrowserThread::PostTaskAndReplyWithResult( + content::BrowserThread::UI, FROM_HERE, + base::Bind(&ExtensionApiFrameIdMap::LookupFrameIdOnUI, + base::Unretained(this), key), + base::Bind(&ExtensionApiFrameIdMap::ReceivedFrameIdOnIO, + base::Unretained(this), key)); +} + +void ExtensionApiFrameIdMap::CacheFrameId(content::RenderFrameHost* rfh) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + + const RenderFrameIdKey key(rfh->GetProcess()->GetID(), rfh->GetRoutingID()); + CacheFrameId(key); + DCHECK(frame_id_map_.find(key) != frame_id_map_.end()); +} + +void ExtensionApiFrameIdMap::CacheFrameId(const RenderFrameIdKey& key) { + LookupFrameIdOnUI(key); +} + +void ExtensionApiFrameIdMap::RemoveFrameId(content::RenderFrameHost* rfh) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + DCHECK(rfh); + + const RenderFrameIdKey key(rfh->GetProcess()->GetID(), rfh->GetRoutingID()); + RemoveFrameId(key); +} + +void ExtensionApiFrameIdMap::RemoveFrameId(const RenderFrameIdKey& key) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + + base::AutoLock lock(frame_id_map_lock_); + frame_id_map_.erase(key); +} + +} // namespace extensions diff --git a/extensions/browser/extension_api_frame_id_map.h b/extensions/browser/extension_api_frame_id_map.h new file mode 100644 index 0000000..c3af8b3 --- /dev/null +++ b/extensions/browser/extension_api_frame_id_map.h @@ -0,0 +1,168 @@ +// Copyright 2015 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 EXTENSIONS_BROWSER_EXTENSION_API_FRAME_ID_MAP_H_ +#define EXTENSIONS_BROWSER_EXTENSION_API_FRAME_ID_MAP_H_ + +#include <list> +#include <map> + +#include "base/callback.h" +#include "base/lazy_instance.h" +#include "base/macros.h" +#include "base/synchronization/lock.h" + +namespace content { +class RenderFrameHost; +class WebContents; +} // namespace content + +namespace extensions { + +// Extension frame IDs are exposed through the chrome.* APIs and have the +// following characteristics: +// - The top-level frame has ID 0. +// - Any child frame has a positive ID. +// - A non-existant frame has ID -1. +// - They are only guaranteed to be unique within a tab. +// - The ID does not change during the frame's lifetime and is not re-used after +// the frame is removed. The frame may change its current RenderFrameHost over +// time, so multiple RenderFrameHosts may map to the same extension frame ID. + +// This class provides a mapping from a (render_process_id, frame_routing_id) +// pair that maps a RenderFrameHost to an extension frame ID. +// Unless stated otherwise, the methods can only be called on the UI thread. +// +// The non-static methods of this class use an internal cache. This cache is +// used to minimize IO->UI->IO round-trips of GetFrameIdOnIO. If the cost of +// attaching FrameTreeNode IDs to requests is negligible (crbug.com/524228), +// then we can remove all key caching and remove the cache from this class. +// TODO(robwu): Keep an eye on crbug.com/524228 and act upon the outcome. +class ExtensionApiFrameIdMap { + public: + using FrameIdCallback = + base::Callback<void(int extension_api_frame_id, + int extension_api_parent_frame_id)>; + + // An invalid extension API frame ID. + static const int kInvalidFrameId; + + static ExtensionApiFrameIdMap* Get(); + + // Get the extension API frame ID for |rfh|. + static int GetFrameId(content::RenderFrameHost* rfh); + + // Get the extension API frame ID for the parent of |rfh|. + static int GetParentFrameId(content::RenderFrameHost* rfh); + + // Find the current RenderFrameHost for a given WebContents and extension + // frame ID. + // Returns nullptr if not found. + static content::RenderFrameHost* GetRenderFrameHostById( + content::WebContents* web_contents, + int frame_id); + + // Runs |callback| with the result that is equivalent to calling GetFrameId() + // on the UI thread. Thread hopping is minimized if possible. Callbacks for + // the same |render_process_id| and |frame_routing_id| are guaranteed to be + // run in order. The order of other callbacks is undefined. + void GetFrameIdOnIO(int render_process_id, + int frame_routing_id, + const FrameIdCallback& callback); + + // Looks up the frame ID and stores it in the map. This method should be + // called as early as possible, e.g. in a + // WebContentsObserver::RenderFrameCreated notification. + void CacheFrameId(content::RenderFrameHost* rfh); + + // Removes the frame ID mapping for a given frame. This method can be called + // at any time, but it is typically called when a frame is destroyed. + // If this method is not called, the cached mapping for the frame is retained + // forever. + void RemoveFrameId(content::RenderFrameHost* rfh); + + protected: + friend struct base::DefaultLazyInstanceTraits<ExtensionApiFrameIdMap>; + + // A set of identifiers that uniquely identifies a RenderFrame. + struct RenderFrameIdKey { + RenderFrameIdKey(); + RenderFrameIdKey(int render_process_id, int frame_routing_id); + + // The process ID of the renderer that contains the RenderFrame. + int render_process_id; + + // The routing ID of the RenderFrame. + int frame_routing_id; + + bool operator<(const RenderFrameIdKey& other) const; + bool operator==(const RenderFrameIdKey& other) const; + }; + + // The cached pair of frame IDs of the frame. Every RenderFrameIdKey + // maps to a CachedFrameIdPair. + struct CachedFrameIdPair { + CachedFrameIdPair(); + CachedFrameIdPair(int frame_id, int parent_frame_id); + + // The extension API frame ID of the frame. + int frame_id; + + // The extension API frame ID of the parent of the frame. + int parent_frame_id; + }; + + struct FrameIdCallbacks { + FrameIdCallbacks(); + ~FrameIdCallbacks(); + + // This is a std::list so that iterators are not invalidated when the list + // is modified during an iteration. + std::list<FrameIdCallback> callbacks; + + // To avoid re-entrant processing of callbacks. + bool is_iterating; + }; + + using FrameIdMap = std::map<RenderFrameIdKey, CachedFrameIdPair>; + using FrameIdCallbacksMap = std::map<RenderFrameIdKey, FrameIdCallbacks>; + + ExtensionApiFrameIdMap(); + ~ExtensionApiFrameIdMap(); + + // Determines the value to be stored in |frame_id_map_| for a given key. This + // method is only called when |key| is not in |frame_id_map_|. + // virtual for testing. + virtual CachedFrameIdPair KeyToValue(const RenderFrameIdKey& key) const; + + CachedFrameIdPair LookupFrameIdOnUI(const RenderFrameIdKey& key); + + // Called as soon as the frame ID is found for the given |key|, and runs all + // queued callbacks with |cached_frame_id_pair|. + void ReceivedFrameIdOnIO(const RenderFrameIdKey& key, + const CachedFrameIdPair& cached_frame_id_pair); + + // Implementation of CacheFrameId(RenderFrameHost), separated for testing. + void CacheFrameId(const RenderFrameIdKey& key); + + // Implementation of RemoveFrameId(RenderFrameHost), separated for testing. + void RemoveFrameId(const RenderFrameIdKey& key); + + // Queued callbacks for use on the IO thread. + FrameIdCallbacksMap callbacks_map_; + + // This map is only modified on the UI thread and is used to minimize the + // number of thread hops on the IO thread. + FrameIdMap frame_id_map_; + + // This lock protects |frame_id_map_| from being concurrently written on the + // UI thread and read on the IO thread. + base::Lock frame_id_map_lock_; + + DISALLOW_COPY_AND_ASSIGN(ExtensionApiFrameIdMap); +}; + +} // namespace extensions + +#endif // EXTENSIONS_BROWSER_EXTENSION_API_FRAME_ID_MAP_H_ diff --git a/extensions/browser/extension_api_frame_id_map_unittest.cc b/extensions/browser/extension_api_frame_id_map_unittest.cc new file mode 100644 index 0000000..119c54d6 --- /dev/null +++ b/extensions/browser/extension_api_frame_id_map_unittest.cc @@ -0,0 +1,222 @@ +// Copyright 2015 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 "base/bind.h" +#include "base/run_loop.h" +#include "content/public/test/test_browser_thread_bundle.h" +#include "extensions/browser/extension_api_frame_id_map.h" +#include "ipc/ipc_message.h" +#include "testing/gtest/include/gtest/gtest.h" + +using FrameIdCallback = extensions::ExtensionApiFrameIdMap::FrameIdCallback; + +namespace extensions { + +namespace { + +int ToTestFrameId(int render_process_id, int frame_routing_id) { + if (render_process_id < 0 && frame_routing_id < 0) + return ExtensionApiFrameIdMap::kInvalidFrameId; + // Return a deterministic value (yet different from the input) for testing. + // To make debugging easier: Ending with 0 = frame ID. + return render_process_id * 1000 + frame_routing_id * 10; +} + +int ToTestParentFrameId(int render_process_id, int frame_routing_id) { + if (render_process_id < 0 && frame_routing_id < 0) + return ExtensionApiFrameIdMap::kInvalidFrameId; + // Return a deterministic value (yet different from the input) for testing. + // To make debugging easier: Ending with 7 = parent frame ID. + return render_process_id * 1000 + frame_routing_id * 10 + 7; +} + +class TestExtensionApiFrameIdMap : public ExtensionApiFrameIdMap { + public: + int GetInternalSize() { return frame_id_map_.size(); } + int GetInternalCallbackCount() { + int count = 0; + for (auto& it : callbacks_map_) + count += it.second.callbacks.size(); + return count; + } + + // These indirections are used because we cannot mock RenderFrameHost with + // fixed IDs in unit tests. + // TODO(robwu): Use content/public/test/test_renderer_host.h to mock + // RenderFrameHosts and update the tests to test against these mocks. + // After doing that, there is no need for CacheFrameId/RemoveFrameId methods + // that take a RenderFrameIdKey, so the methods can be merged. + void SetInternalFrameId(int render_process_id, int frame_routing_id) { + CacheFrameId(RenderFrameIdKey(render_process_id, frame_routing_id)); + } + void RemoveInternalFrameId(int render_process_id, int frame_routing_id) { + RemoveFrameId(RenderFrameIdKey(render_process_id, frame_routing_id)); + } + + private: + // ExtensionApiFrameIdMap: + CachedFrameIdPair KeyToValue(const RenderFrameIdKey& key) const override { + return CachedFrameIdPair( + ToTestFrameId(key.render_process_id, key.frame_routing_id), + ToTestParentFrameId(key.render_process_id, key.frame_routing_id)); + } +}; + +class ExtensionApiFrameIdMapTest : public testing::Test { + public: + ExtensionApiFrameIdMapTest() + : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) {} + + FrameIdCallback CreateCallback(int render_process_id, + int frame_routing_id, + const std::string& callback_name_for_testing) { + return base::Bind(&ExtensionApiFrameIdMapTest::OnCalledCallback, + base::Unretained(this), render_process_id, + frame_routing_id, callback_name_for_testing); + } + + void OnCalledCallback(int render_process_id, + int frame_routing_id, + const std::string& callback_name_for_testing, + int extension_api_frame_id, + int extension_api_parent_frame_id) { + results_.push_back(callback_name_for_testing); + + // If this fails, then the mapping is completely wrong. + EXPECT_EQ(ToTestFrameId(render_process_id, frame_routing_id), + extension_api_frame_id); + EXPECT_EQ(ToTestParentFrameId(render_process_id, frame_routing_id), + extension_api_parent_frame_id); + } + + const std::vector<std::string>& results() { return results_; } + void ClearResults() { results_.clear(); } + + private: + content::TestBrowserThreadBundle thread_bundle_; + // Used to verify the order of callbacks. + std::vector<std::string> results_; + + DISALLOW_COPY_AND_ASSIGN(ExtensionApiFrameIdMapTest); +}; + +} // namespace + +TEST_F(ExtensionApiFrameIdMapTest, GetFrameIdOnIO) { + TestExtensionApiFrameIdMap map; + EXPECT_EQ(0, map.GetInternalSize()); + + // Two identical calls, should be processed at the next message loop. + map.GetFrameIdOnIO(1, 2, CreateCallback(1, 2, "first")); + EXPECT_EQ(1, map.GetInternalCallbackCount()); + EXPECT_EQ(0, map.GetInternalSize()); + + map.GetFrameIdOnIO(1, 2, CreateCallback(1, 2, "first again")); + EXPECT_EQ(2, map.GetInternalCallbackCount()); + EXPECT_EQ(0, map.GetInternalSize()); + + // First get the frame ID on IO (queued on message loop), then set it on UI. + // No callbacks should be invoked because the IO thread cannot know that the + // frame ID was set on the UI thread. + map.GetFrameIdOnIO(2, 1, CreateCallback(2, 1, "something else")); + EXPECT_EQ(3, map.GetInternalCallbackCount()); + EXPECT_EQ(0, map.GetInternalSize()); + + map.SetInternalFrameId(2, 1); + EXPECT_EQ(1, map.GetInternalSize()); + EXPECT_EQ(0U, results().size()); + + // Run some self-contained test. They should not affect the above callbacks. + { + // Callbacks for invalid IDs should immediately be run because it doesn't + // require a thread hop to determine their invalidity. + map.GetFrameIdOnIO(-1, MSG_ROUTING_NONE, + CreateCallback(-1, MSG_ROUTING_NONE, "invalid IDs")); + EXPECT_EQ(3, map.GetInternalCallbackCount()); // No change. + EXPECT_EQ(1, map.GetInternalSize()); // No change. + ASSERT_EQ(1U, results().size()); // +1 + EXPECT_EQ("invalid IDs", results()[0]); + ClearResults(); + } + + { + // First set the frame ID on UI, then get it on IO. Callback should + // immediately be invoked. + map.SetInternalFrameId(3, 1); + EXPECT_EQ(2, map.GetInternalSize()); + + map.GetFrameIdOnIO(3, 1, CreateCallback(3, 1, "the only result")); + EXPECT_EQ(3, map.GetInternalCallbackCount()); // No change. + EXPECT_EQ(2, map.GetInternalSize()); // +1 + ASSERT_EQ(1U, results().size()); // +1 + EXPECT_EQ("the only result", results()[0]); + ClearResults(); + } + + { + // Request the frame ID on IO, set the frame ID (in reality, set on the UI), + // and request another frame ID. The last query should cause both callbacks + // to run because the frame ID is known at the time of the call. + map.GetFrameIdOnIO(7, 2, CreateCallback(7, 2, "queued")); + EXPECT_EQ(4, map.GetInternalCallbackCount()); // +1 + + map.SetInternalFrameId(7, 2); + EXPECT_EQ(3, map.GetInternalSize()); // +1 + + map.GetFrameIdOnIO(7, 2, CreateCallback(7, 2, "not queued")); + EXPECT_EQ(3, map.GetInternalCallbackCount()); // -1 (first callback ran). + EXPECT_EQ(3, map.GetInternalSize()); // No change. + ASSERT_EQ(2U, results().size()); // +2 (both callbacks ran). + EXPECT_EQ("queued", results()[0]); + EXPECT_EQ("not queued", results()[1]); + ClearResults(); + } + + // A call identical to the very first call. + map.GetFrameIdOnIO(1, 2, CreateCallback(1, 2, "same as first")); + EXPECT_EQ(4, map.GetInternalCallbackCount()); + EXPECT_EQ(3, map.GetInternalSize()); + + // Trigger the queued callbacks. + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(0, map.GetInternalCallbackCount()); // -4 (no queued callbacks). + + EXPECT_EQ(4, map.GetInternalSize()); // +1 (1 new cached frame ID). + ASSERT_EQ(4U, results().size()); // +4 (callbacks ran). + + // PostTasks are processed in order, so the very first callbacks should be + // processed. As soon as the first callback is available, all of its callbacks + // should be run (no deferrals!). + EXPECT_EQ("first", results()[0]); + EXPECT_EQ("first again", results()[1]); + EXPECT_EQ("same as first", results()[2]); + // This was queued after "first again", but has a different frame ID, so it + // is received after "same as first". + EXPECT_EQ("something else", results()[3]); + ClearResults(); + + // Request the frame ID for input that was already looked up. Should complete + // synchronously. + map.GetFrameIdOnIO(1, 2, CreateCallback(1, 2, "first and cached")); + EXPECT_EQ(0, map.GetInternalCallbackCount()); // No change. + EXPECT_EQ(4, map.GetInternalSize()); // No change. + ASSERT_EQ(1U, results().size()); // +1 (synchronous callback). + EXPECT_EQ("first and cached", results()[0]); + ClearResults(); + + // Trigger frame removal and look up frame ID. The frame ID should no longer + // be available. and GetFrameIdOnIO() should require a thread hop. + map.RemoveInternalFrameId(1, 2); + EXPECT_EQ(3, map.GetInternalSize()); // -1 + map.GetFrameIdOnIO(1, 2, CreateCallback(1, 2, "first was removed")); + EXPECT_EQ(1, map.GetInternalCallbackCount()); // +1 + ASSERT_EQ(0U, results().size()); // No change (queued callback). + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(0, map.GetInternalCallbackCount()); // -1 (callback not in queue). + EXPECT_EQ(4, map.GetInternalSize()); // +1 (cached frame ID). + ASSERT_EQ(1U, results().size()); // +1 (callback ran). + EXPECT_EQ("first was removed", results()[0]); +} + +} // namespace extensions diff --git a/extensions/browser/extension_web_contents_observer.cc b/extensions/browser/extension_web_contents_observer.cc index 757c1af..ac482df 100644 --- a/extensions/browser/extension_web_contents_observer.cc +++ b/extensions/browser/extension_web_contents_observer.cc @@ -12,6 +12,7 @@ #include "content/public/browser/site_instance.h" #include "content/public/browser/web_contents.h" #include "content/public/common/url_constants.h" +#include "extensions/browser/extension_api_frame_id_map.h" #include "extensions/browser/extension_prefs.h" #include "extensions/browser/extension_registry.h" #include "extensions/browser/extensions_browser_client.h" @@ -110,12 +111,18 @@ void ExtensionWebContentsObserver::RenderViewCreated( void ExtensionWebContentsObserver::RenderFrameCreated( content::RenderFrameHost* render_frame_host) { InitializeRenderFrame(render_frame_host); + + // Optimization: Look up the extension API frame ID to force the mapping to be + // cached. This minimizes the number of IO->UI->IO thread hops when the ID is + // looked up again on the IO thread for the webRequest API. + ExtensionApiFrameIdMap::Get()->CacheFrameId(render_frame_host); } void ExtensionWebContentsObserver::RenderFrameDeleted( content::RenderFrameHost* render_frame_host) { ProcessManager::Get(browser_context_) ->UnregisterRenderFrameHost(render_frame_host); + ExtensionApiFrameIdMap::Get()->RemoveFrameId(render_frame_host); } void ExtensionWebContentsObserver::DidCommitProvisionalLoadForFrame( diff --git a/extensions/common/extension_messages.h b/extensions/common/extension_messages.h index ce8f251..6ff3cf1 100644 --- a/extensions/common/extension_messages.h +++ b/extensions/common/extension_messages.h @@ -182,14 +182,6 @@ IPC_STRUCT_BEGIN(ExtensionMsg_ExternalConnectionInfo) // The URL of the frame that initiated the request. IPC_STRUCT_MEMBER(GURL, source_url) - // The ID of the tab that is the target of the request, or -1 if there is no - // target tab. - IPC_STRUCT_MEMBER(int, target_tab_id) - - // The ID of the frame that is the target of the request, or -1 if there is - // no target frame (implying the message is for all frames). - IPC_STRUCT_MEMBER(int, target_frame_id) - // The process ID of the webview that initiated the request. IPC_STRUCT_MEMBER(int, guest_process_id) @@ -642,38 +634,46 @@ IPC_MESSAGE_ROUTED1(ExtensionHostMsg_EventAck, int /* message_id */) // sending messages. If an error occurred, the opener will be notified // asynchronously. IPC_SYNC_MESSAGE_CONTROL4_1(ExtensionHostMsg_OpenChannelToExtension, - int /* routing_id */, + int /* frame_routing_id */, ExtensionMsg_ExternalConnectionInfo, std::string /* channel_name */, bool /* include_tls_channel_id */, int /* port_id */) IPC_SYNC_MESSAGE_CONTROL3_1(ExtensionHostMsg_OpenChannelToNativeApp, - int /* routing_id */, + int /* frame_routing_id */, std::string /* source_extension_id */, std::string /* native_app_name */, int /* port_id */) // Get a port handle to the given tab. The handle can be used for sending // messages to the extension. -IPC_SYNC_MESSAGE_CONTROL3_1(ExtensionHostMsg_OpenChannelToTab, +IPC_SYNC_MESSAGE_CONTROL4_1(ExtensionHostMsg_OpenChannelToTab, + int /* frame_routing_id */, ExtensionMsg_TabTargetConnectionInfo, std::string /* extension_id */, std::string /* channel_name */, int /* port_id */) +// Sent in response to ExtensionMsg_DispatchOnConnect when the port is accepted. +// The handle is the value returned by ExtensionHostMsg_OpenChannelTo*. +IPC_MESSAGE_CONTROL2(ExtensionHostMsg_OpenMessagePort, + int /* frame_routing_id */, + int /* port_id */); + +// Sent in response to ExtensionMsg_DispatchOnConnect and whenever the port is +// closed. The handle is the value returned by ExtensionHostMsg_OpenChannelTo*. +IPC_MESSAGE_CONTROL3(ExtensionHostMsg_CloseMessagePort, + int /* frame_routing_id */, + int /* port_id */, + bool /* force_close */); + // Send a message to an extension process. The handle is the value returned -// by ViewHostMsg_OpenChannelTo*. +// by ExtensionHostMsg_OpenChannelTo*. IPC_MESSAGE_ROUTED2(ExtensionHostMsg_PostMessage, int /* port_id */, extensions::Message) -// Send a message to an extension process. The handle is the value returned -// by ViewHostMsg_OpenChannelTo*. -IPC_MESSAGE_CONTROL2(ExtensionHostMsg_CloseChannel, - int /* port_id */, - std::string /* error_message */) - // Used to get the extension message bundle. IPC_SYNC_MESSAGE_CONTROL1_1(ExtensionHostMsg_GetMessageBundle, std::string /* extension id */, diff --git a/extensions/extensions.gypi b/extensions/extensions.gypi index e18e682..b49e007 100644 --- a/extensions/extensions.gypi +++ b/extensions/extensions.gypi @@ -565,6 +565,8 @@ 'browser/event_router.h', 'browser/event_router_factory.cc', 'browser/event_router_factory.h', + 'browser/extension_api_frame_id_map.cc', + 'browser/extension_api_frame_id_map.h', 'browser/extension_dialog_auto_confirm.cc', 'browser/extension_dialog_auto_confirm.h', 'browser/extension_error.cc', diff --git a/extensions/extensions_tests.gypi b/extensions/extensions_tests.gypi index e164a8c..2c7209a 100644 --- a/extensions/extensions_tests.gypi +++ b/extensions/extensions_tests.gypi @@ -77,6 +77,7 @@ 'browser/error_map_unittest.cc', 'browser/event_listener_map_unittest.cc', 'browser/event_router_unittest.cc', + 'browser/extension_api_frame_id_map_unittest.cc', 'browser/extension_icon_image_unittest.cc', 'browser/extension_pref_value_map_unittest.cc', 'browser/extension_registry_unittest.cc', diff --git a/extensions/renderer/messaging_bindings.cc b/extensions/renderer/messaging_bindings.cc index a0a855f..722bc72 100644 --- a/extensions/renderer/messaging_bindings.cc +++ b/extensions/renderer/messaging_bindings.cc @@ -130,8 +130,6 @@ class PortTracker { base::LazyInstance<PortTracker> g_port_tracker = LAZY_INSTANCE_INITIALIZER; const char kPortClosedError[] = "Attempting to use a disconnected port object"; -const char kReceivingEndDoesntExistError[] = - "Could not establish connection. Receiving end does not exist."; class ExtensionImpl : public ObjectBackedNativeHandler { public: @@ -175,8 +173,8 @@ class ExtensionImpl : public ObjectBackedNativeHandler { // Sends a message along the given channel. void PostMessage(const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderFrame* renderframe = context()->GetRenderFrame(); - if (!renderframe) + content::RenderFrame* render_frame = context()->GetRenderFrame(); + if (!render_frame) return; // Arguments are (int32_t port_id, string message). @@ -190,8 +188,8 @@ class ExtensionImpl : public ObjectBackedNativeHandler { return; } - renderframe->Send(new ExtensionHostMsg_PostMessage( - renderframe->GetRoutingID(), port_id, + render_frame->Send(new ExtensionHostMsg_PostMessage( + render_frame->GetRoutingID(), port_id, Message(*v8::String::Utf8Value(args[1]), blink::WebUserGestureIndicator::isProcessingUserGesture()))); } @@ -209,9 +207,10 @@ class ExtensionImpl : public ObjectBackedNativeHandler { // Send via the RenderThread because the RenderFrame might be closing. bool notify_browser = args[1].As<v8::Boolean>()->Value(); - if (notify_browser) { - content::RenderThread::Get()->Send( - new ExtensionHostMsg_CloseChannel(port_id, std::string())); + content::RenderFrame* render_frame = context()->GetRenderFrame(); + if (notify_browser && render_frame) { + render_frame->Send(new ExtensionHostMsg_CloseMessagePort( + render_frame->GetRoutingID(), port_id, true)); } ClearPortDataAndNotifyDispatcher(port_id); @@ -231,6 +230,8 @@ class ExtensionImpl : public ObjectBackedNativeHandler { // The frame a port lived in has been destroyed. When there are no more // frames with a reference to a given port, we will disconnect it and notify // the other end of the channel. + // TODO(robwu): Port lifetime management has moved to the browser, this is no + // longer needed. See .destroy_() inmessaging.js for more details. void PortRelease(const v8::FunctionCallbackInfo<v8::Value>& args) { // Arguments are (int32_t port_id). CHECK(args.Length() == 1 && args[0]->IsInt32()); @@ -240,12 +241,11 @@ class ExtensionImpl : public ObjectBackedNativeHandler { // Releases the reference to |port_id| for this context, and clears all port // data if there are no more references. void ReleasePort(int port_id) { + content::RenderFrame* render_frame = context()->GetRenderFrame(); if (g_port_tracker.Get().RemoveReference(context(), port_id) && - !g_port_tracker.Get().HasPort(port_id)) { - // Send via the RenderThread because the RenderFrame might be closing. - content::RenderThread::Get()->Send( - new ExtensionHostMsg_CloseChannel(port_id, std::string())); - ClearPortDataAndNotifyDispatcher(port_id); + !g_port_tracker.Get().HasPort(port_id) && render_frame) { + render_frame->Send(new ExtensionHostMsg_CloseMessagePort( + render_frame->GetRoutingID(), port_id, false)); } } @@ -286,23 +286,6 @@ void DispatchOnConnectToScriptContext( const std::string& tls_channel_id, bool* port_created, ScriptContext* script_context) { - // Only dispatch the events if this is the requested target frame (0 = main - // frame; positive = child frame). - content::RenderFrame* renderframe = script_context->GetRenderFrame(); - if (info.target_frame_id == 0 && renderframe->GetWebFrame()->parent() != NULL) - return; - if (info.target_frame_id > 0 && - renderframe->GetRoutingID() != info.target_frame_id) - return; - - // Bandaid fix for crbug.com/520303. - // TODO(rdevlin.cronin): Fix this properly by routing messages to the correct - // RenderFrame from the browser (same with |target_frame_id| in fact). - if (info.target_tab_id != -1 && - info.target_tab_id != ExtensionFrameHelper::Get(renderframe)->tab_id()) { - return; - } - v8::Isolate* isolate = script_context->isolate(); v8::HandleScope handle_scope(isolate); @@ -472,11 +455,15 @@ void MessagingBindings::DispatchOnConnect( base::Bind(&DispatchOnConnectToScriptContext, target_port_id, channel_name, &source, info, tls_channel_id, &port_created)); - // If we didn't create a port, notify the other end of the channel (treat it - // as a disconnect). - if (!port_created) { - content::RenderThread::Get()->Send(new ExtensionHostMsg_CloseChannel( - target_port_id, kReceivingEndDoesntExistError)); + int routing_id = restrict_to_render_frame + ? restrict_to_render_frame->GetRoutingID() + : MSG_ROUTING_NONE; + if (port_created) { + content::RenderThread::Get()->Send( + new ExtensionHostMsg_OpenMessagePort(routing_id, target_port_id)); + } else { + content::RenderThread::Get()->Send(new ExtensionHostMsg_CloseMessagePort( + routing_id, target_port_id, false)); } } diff --git a/extensions/renderer/resources/messaging.js b/extensions/renderer/resources/messaging.js index 5a7ec1e..a15497d 100644 --- a/extensions/renderer/resources/messaging.js +++ b/extensions/renderer/resources/messaging.js @@ -76,6 +76,16 @@ this.onDestroy_(); privates(this.onDisconnect).impl.destroy_(); privates(this.onMessage).impl.destroy_(); + // TODO(robwu): Remove port lifetime management because it is completely + // handled in the browser. The renderer's only roles are + // 1) rejecting ports so that the browser knows that the renderer is not + // interested in the port (this is merely an optimization) + // 2) acknowledging port creations, so that the browser knows that the port + // was successfully created (from the perspective of the extension), but + // then closed for some non-fatal reason. + // 3) notifying the browser of explicit port closure via .disconnect(). + // In other cases (navigations), the browser automatically cleans up the + // port. messagingNatives.PortRelease(this.portId_); delete ports[this.portId_]; }; |