summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormpcomplete@chromium.org <mpcomplete@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-07-27 19:18:41 +0000
committermpcomplete@chromium.org <mpcomplete@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-07-27 19:18:41 +0000
commitcc661e5f76c24a8640dbe818a1091e0fe60e1529 (patch)
tree09a8c5c6c76a4c5c990eb1a3fe862373d9536ebe
parent1a16364587d4fe3592b78d04ac13b5abfbd6360e (diff)
downloadchromium_src-cc661e5f76c24a8640dbe818a1091e0fe60e1529.zip
chromium_src-cc661e5f76c24a8640dbe818a1091e0fe60e1529.tar.gz
chromium_src-cc661e5f76c24a8640dbe818a1091e0fe60e1529.tar.bz2
Fix port disconnect so that it's refcounted. This fixes a bug where a channel
would close if any one listener on the channel called "disconnect()". BUG=16644 TEST=no Review URL: http://codereview.chromium.org/155476 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@21678 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat
-rw-r--r--chrome/browser/extensions/extension_messages_unittest.cc69
-rw-r--r--chrome/renderer/extensions/renderer_extension_bindings.cc45
-rw-r--r--chrome/renderer/renderer_resources.grd2
-rw-r--r--chrome/renderer/resources/renderer_extension_bindings.js5
-rw-r--r--chrome/test/in_process_browser_test.cc7
5 files changed, 122 insertions, 6 deletions
diff --git a/chrome/browser/extensions/extension_messages_unittest.cc b/chrome/browser/extensions/extension_messages_unittest.cc
index 7da3f38..7bf402f 100644
--- a/chrome/browser/extensions/extension_messages_unittest.cc
+++ b/chrome/browser/extensions/extension_messages_unittest.cc
@@ -147,3 +147,72 @@ TEST_F(RenderViewTest, ExtensionMessagesOnConnect) {
ASSERT_TRUE(IPC::ReadParam(alert_msg, &iter, &alert_param));
EXPECT_EQ(L"disconnected: 24", alert_param.a);
}
+
+// Tests that we don't send the disconnect message until all ports have
+// disconnected.
+TEST_F(RenderViewTest, ExtensionMessagesDisconnect) {
+ LoadHTML("<body></body>");
+ ExecuteJavaScript(
+ "chrome.self.onConnect.addListener(function (port) {"
+ " port.onMessage.addListener(function(msg, p) {"
+ " if (msg.disconnect) port.disconnect();"
+ " });"
+ "});"
+ "var iframe1 = document.createElement('iframe');"
+ "var iframe2 = document.createElement('iframe');"
+ "var src = 'javascript:"
+ " chrome.self.onConnect.addListener(function(port) {"
+ " port.postMessage(\"onconnect\");"
+ " port.onMessage.addListener(function(p) { alert(\"NOTREACHED\"); });"
+ " port.disconnect();"
+ " });';"
+ "iframe1.src = src;"
+ "iframe2.src = src;"
+ "document.body.appendChild(iframe1);"
+ "document.body.appendChild(iframe2);");
+
+ render_thread_.sink().ClearMessages();
+
+ // Simulate a new connection being opened. The two child frames should
+ // disconnect immediately, but we shouldn't get a disconnect message until
+ // all 3 frames disconnect.
+ const int kPortId = 0;
+ const std::string kPortName = "testName";
+ DispatchOnConnect(kPortId, kPortName, "null");
+
+ // Verify that we handled the new connection by posting a message.
+ const IPC::Message* post_msg =
+ render_thread_.sink().GetFirstMessageMatching(
+ ViewHostMsg_ExtensionPostMessage::ID);
+ ASSERT_TRUE(post_msg);
+ ViewHostMsg_ExtensionPostMessage::Param post_params;
+ ViewHostMsg_ExtensionPostMessage::Read(post_msg, &post_params);
+ EXPECT_EQ("\"onconnect\"", post_params.b);
+
+ // Simulate sending a message.
+ render_thread_.sink().ClearMessages();
+ DispatchOnMessage("{\"val\": 42}", kPortId);
+
+ // If we get an alert box, then the iframes failed to disconnect properly.
+ const IPC::Message* alert_msg =
+ render_thread_.sink().GetFirstMessageMatching(
+ ViewHostMsg_RunJavaScriptMessage::ID);
+ ASSERT_FALSE(alert_msg);
+
+ // We should not get a disconnect message yet, since the toplevel frame didn't
+ // disconnect.
+ const IPC::Message* disconnect_msg =
+ render_thread_.sink().GetFirstMessageMatching(
+ ViewHostMsg_ExtensionCloseChannel::ID);
+ ASSERT_FALSE(disconnect_msg);
+
+ // Disconnect the port in the top frame.
+ render_thread_.sink().ClearMessages();
+ DispatchOnMessage("{\"disconnect\": true}", kPortId);
+
+ // Now we should have a disconnect message.
+ disconnect_msg =
+ render_thread_.sink().GetUniqueMessageMatching(
+ ViewHostMsg_ExtensionCloseChannel::ID);
+ ASSERT_TRUE(disconnect_msg);
+}
diff --git a/chrome/renderer/extensions/renderer_extension_bindings.cc b/chrome/renderer/extensions/renderer_extension_bindings.cc
index 9de9035..2facea6 100644
--- a/chrome/renderer/extensions/renderer_extension_bindings.cc
+++ b/chrome/renderer/extensions/renderer_extension_bindings.cc
@@ -29,6 +29,18 @@ using bindings_utils::ExtensionBase;
namespace {
+struct ExtensionData {
+ struct PortData {
+ int ref_count; // how many contexts have a handle to this port
+ bool disconnected; // true if this port was forcefully disconnected
+ PortData() : ref_count(0), disconnected(false) {}
+ };
+ std::map<int, PortData> ports; // port ID -> data
+};
+ExtensionData::PortData& GetPortData(int port_id) {
+ return Singleton<ExtensionData>::get()->ports[port_id];
+}
+
const char* kExtensionDeps[] = { EventBindings::kName };
class ExtensionImpl : public ExtensionBase {
@@ -48,6 +60,10 @@ class ExtensionImpl : public ExtensionBase {
return v8::FunctionTemplate::New(PostMessage);
} else if (name->Equals(v8::String::New("CloseChannel"))) {
return v8::FunctionTemplate::New(CloseChannel);
+ } else if (name->Equals(v8::String::New("PortAddRef"))) {
+ return v8::FunctionTemplate::New(PortAddRef);
+ } else if (name->Equals(v8::String::New("PortRelease"))) {
+ return v8::FunctionTemplate::New(PortRelease);
}
return ExtensionBase::GetNativeFunction(name);
}
@@ -87,13 +103,40 @@ class ExtensionImpl : public ExtensionBase {
return v8::Undefined();
}
- // Sends a message along the given channel.
+ // Forcefully disconnects a port.
static v8::Handle<v8::Value> CloseChannel(const v8::Arguments& args) {
if (args.Length() >= 1 && args[0]->IsInt32()) {
int port_id = args[0]->Int32Value();
// Send via the RenderThread because the RenderView might be closing.
EventBindings::GetRenderThread()->Send(
new ViewHostMsg_ExtensionCloseChannel(port_id));
+ GetPortData(port_id).disconnected = true;
+ }
+ return v8::Undefined();
+ }
+
+ // A new port has been created for a context. This occurs both when script
+ // opens a connection, and when a connection is opened to this script.
+ static v8::Handle<v8::Value> PortAddRef(const v8::Arguments& args) {
+ if (args.Length() >= 1 && args[0]->IsInt32()) {
+ int port_id = args[0]->Int32Value();
+ ++GetPortData(port_id).ref_count;
+ }
+ return v8::Undefined();
+ }
+
+ // 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.
+ static v8::Handle<v8::Value> PortRelease(const v8::Arguments& args) {
+ if (args.Length() >= 1 && args[0]->IsInt32()) {
+ int port_id = args[0]->Int32Value();
+ if (!GetPortData(port_id).disconnected &&
+ --GetPortData(port_id).ref_count == 0) {
+ // Send via the RenderThread because the RenderView might be closing.
+ EventBindings::GetRenderThread()->Send(
+ new ViewHostMsg_ExtensionCloseChannel(port_id));
+ }
}
return v8::Undefined();
}
diff --git a/chrome/renderer/renderer_resources.grd b/chrome/renderer/renderer_resources.grd
index e5c7fcf..bdba06d 100644
--- a/chrome/renderer/renderer_resources.grd
+++ b/chrome/renderer/renderer_resources.grd
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<!-- This comment is only here because changes to resources are not picked up
-without changes to the corresponding grd file. rw2 -->
+without changes to the corresponding grd file. mp1 -->
<grit latest_public_release="0" current_release="1">
<outputs>
<output filename="grit/renderer_resources.h" type="rc_header">
diff --git a/chrome/renderer/resources/renderer_extension_bindings.js b/chrome/renderer/resources/renderer_extension_bindings.js
index d8b5a4b..cf53de9 100644
--- a/chrome/renderer/resources/renderer_extension_bindings.js
+++ b/chrome/renderer/resources/renderer_extension_bindings.js
@@ -18,6 +18,8 @@ var chrome = chrome || {};
(function () {
native function OpenChannelToExtension(id);
native function CloseChannel(portId);
+ native function PortAddRef(portId);
+ native function PortRelease(portId);
native function PostMessage(portId, msg);
native function GetChromeHidden();
@@ -46,8 +48,9 @@ var chrome = chrome || {};
var port = new chrome.Port(portId, opt_name);
ports[portId] = port;
chromeHidden.onUnload.addListener(function() {
- port.disconnect();
+ PortRelease(portId);
});
+ PortAddRef(portId);
return port;
}
diff --git a/chrome/test/in_process_browser_test.cc b/chrome/test/in_process_browser_test.cc
index 36deb0b..36f2977 100644
--- a/chrome/test/in_process_browser_test.cc
+++ b/chrome/test/in_process_browser_test.cc
@@ -235,14 +235,15 @@ void InProcessBrowserTest::RunTestOnMainThreadLoop() {
RunTestOnMainThread();
CleanUpOnMainThread();
+ // Close all browser windows. This might not happen immediately, since some
+ // may need to wait for beforeunload and unload handlers to fire in a tab.
+ // When all windows are closed, the last window will call Quit().
BrowserList::const_iterator browser = BrowserList::begin();
for (; browser != BrowserList::end(); ++browser)
- (*browser)->CloseAllTabs();
+ (*browser)->CloseWindow();
// Stop the HTTP server.
http_server_ = NULL;
-
- MessageLoopForUI::current()->Quit();
}
void InProcessBrowserTest::ConfigureHostResolverProc(
generated by cgit v1.1 at 2025-01-18 00:22:24 +0000