summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-09-11 21:01:17 +0000
committerananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-09-11 21:01:17 +0000
commit91332b626ed0625764b76ec108f5de24ea7c22ed (patch)
tree5d6d40632c7b4de720ae3dd96431caf27d17aba5
parent02473008f5f1948494b9fb69e5781746b66f6f80 (diff)
downloadchromium_src-91332b626ed0625764b76ec108f5de24ea7c22ed.zip
chromium_src-91332b626ed0625764b76ec108f5de24ea7c22ed.tar.gz
chromium_src-91332b626ed0625764b76ec108f5de24ea7c22ed.tar.bz2
Fixes a crash in the plugin process specifically in the PluginStreamUrl::DidReceiveData
function. This crash occurs if the PluginStreamUrl instance is deleted in the context of PluginStream::Write which occurs if the underlying NPP_Write call to the plugin returns a negative value, indicating that the plugin did not accept data. We close the stream in this case which releases existing references to the PluginStream which results in the object being deleted. Added a UI test for this case. Fixes bug http://code.google.com/p/chromium/issues/detail?id=19393 Bug=19393 Review URL: http://codereview.chromium.org/199093 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@26011 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/test/data/npapi/plugin_read_page.html4
-rw-r--r--chrome/test/data/npapi/plugin_read_page.html.mock-http-headers2
-rw-r--r--chrome/test/data/npapi/plugin_url_request_fail_write.html26
-rw-r--r--chrome/test/ui/npapi_uitest.cc13
-rw-r--r--webkit/glue/plugins/plugin_stream_url.cc9
-rw-r--r--webkit/glue/plugins/test/plugin_client.cc3
-rw-r--r--webkit/glue/plugins/test/plugin_geturl_test.cc19
-rw-r--r--webkit/glue/plugins/test/plugin_geturl_test.h1
8 files changed, 73 insertions, 4 deletions
diff --git a/chrome/test/data/npapi/plugin_read_page.html b/chrome/test/data/npapi/plugin_read_page.html
new file mode 100644
index 0000000..17bcef8
--- /dev/null
+++ b/chrome/test/data/npapi/plugin_read_page.html
@@ -0,0 +1,4 @@
+<html>
+<head><title>Test page read by plugin</title></head>
+<body>This page has a title.</body>
+</html>
diff --git a/chrome/test/data/npapi/plugin_read_page.html.mock-http-headers b/chrome/test/data/npapi/plugin_read_page.html.mock-http-headers
new file mode 100644
index 0000000..57fae25
--- /dev/null
+++ b/chrome/test/data/npapi/plugin_read_page.html.mock-http-headers
@@ -0,0 +1,2 @@
+HTTP/1.0 200 OK
+Content-type: text/html
diff --git a/chrome/test/data/npapi/plugin_url_request_fail_write.html b/chrome/test/data/npapi/plugin_url_request_fail_write.html
new file mode 100644
index 0000000..ee55af6
--- /dev/null
+++ b/chrome/test/data/npapi/plugin_url_request_fail_write.html
@@ -0,0 +1,26 @@
+<html>
+
+<head>
+<script src="npapi.js"></script>
+</head>
+
+
+<body>
+<div id="statusPanel" style="border: 1px solid red; width: 100%">
+Test running....
+</div>
+
+
+GetURL Plugin Fails NPP_Write Test<p>
+This test fetches a URL and passes data off to the plugin. The plugin returns
+an error from the NPP_Write call. This test verifies that we do not crash
+as a result.
+
+<embed type="application/vnd.npapi-test"
+ fail_write_url="http://mock.http/npapi/plugin_read_page.html"
+ name="geturl_fail_write"
+ id="1"
+ mode="np_embed"
+>
+</body>
+</html>
diff --git a/chrome/test/ui/npapi_uitest.cc b/chrome/test/ui/npapi_uitest.cc
index 6d1361d..32139fa 100644
--- a/chrome/test/ui/npapi_uitest.cc
+++ b/chrome/test/ui/npapi_uitest.cc
@@ -16,6 +16,7 @@
#include <ostream>
#include "base/file_util.h"
+#include "chrome/browser/net/url_request_mock_http_job.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/test/automation/tab_proxy.h"
#include "chrome/test/automation/window_proxy.h"
@@ -297,3 +298,15 @@ TEST_F(NPAPIVisiblePluginTester, MultipleInstancesSyncCalls) {
kTestCompleteSuccess, kShortWaitTimeout);
}
+TEST_F(NPAPIVisiblePluginTester, GetURLRequestFailWrite) {
+ if (UITest::in_process_renderer())
+ return;
+
+ GURL url(URLRequestMockHTTPJob::GetMockUrl(
+ L"npapi/plugin_url_request_fail_write.html"));
+
+ NavigateToURL(url);
+
+ WaitForFinish("geturl_fail_write", "1", url, kTestCompleteCookie,
+ kTestCompleteSuccess, kShortWaitTimeout);
+}
diff --git a/webkit/glue/plugins/plugin_stream_url.cc b/webkit/glue/plugins/plugin_stream_url.cc
index 2aa2f84..ae26e62 100644
--- a/webkit/glue/plugins/plugin_stream_url.cc
+++ b/webkit/glue/plugins/plugin_stream_url.cc
@@ -65,9 +65,12 @@ void PluginStreamUrl::DidReceiveData(const char* buffer, int length,
return;
if (length > 0) {
- Write(const_cast<char*>(buffer), length, data_offset);
- if (id_ > 0)
- instance()->webplugin()->SetDeferResourceLoading(id_, false);
+ // The PluginStreamUrl instance could get deleted if the plugin fails to
+ // accept data in NPP_Write.
+ if (Write(const_cast<char*>(buffer), length, data_offset) > 0) {
+ if (id_ > 0)
+ instance()->webplugin()->SetDeferResourceLoading(id_, false);
+ }
}
}
diff --git a/webkit/glue/plugins/test/plugin_client.cc b/webkit/glue/plugins/test/plugin_client.cc
index 2361805..66e41b8 100644
--- a/webkit/glue/plugins/test/plugin_client.cc
+++ b/webkit/glue/plugins/test/plugin_client.cc
@@ -99,7 +99,8 @@ NPError NPP_New(NPMIMEType pluginType, NPP instance, uint16 mode,
if (test_name == "arguments") {
new_test = new NPAPIClient::PluginArgumentsTest(instance,
NPAPIClient::PluginClient::HostFunctions());
- } else if (test_name == "geturl" || test_name == "geturl_404_response") {
+ } else if (test_name == "geturl" || test_name == "geturl_404_response" ||
+ test_name == "geturl_fail_write") {
new_test = new NPAPIClient::PluginGetURLTest(instance,
NPAPIClient::PluginClient::HostFunctions());
} else if (test_name == "npobject_proxy") {
diff --git a/webkit/glue/plugins/test/plugin_geturl_test.cc b/webkit/glue/plugins/test/plugin_geturl_test.cc
index 1a9e440..baf9080 100644
--- a/webkit/glue/plugins/test/plugin_geturl_test.cc
+++ b/webkit/glue/plugins/test/plugin_geturl_test.cc
@@ -46,6 +46,12 @@ NPError PluginGetURLTest::New(uint16 mode, int16 argc, const char* argn[],
if (page_not_found_url) {
page_not_found_url_ = page_not_found_url;
expect_404_response_ = true;
+ } else {
+ const char* fail_write_url = GetArgValue("fail_write_url", argc,
+ argn, argv);
+ if (fail_write_url) {
+ fail_write_url_ = fail_write_url;
+ }
}
return PluginTest::New(mode, argc, argn, argv, saved);
@@ -60,6 +66,9 @@ NPError PluginGetURLTest::SetWindow(NPWindow* pNPWindow) {
if (expect_404_response_) {
HostFunctions()->geturl(id(), page_not_found_url_.c_str(), NULL);
return NPERR_NO_ERROR;
+ } else if (!fail_write_url_.empty()) {
+ HostFunctions()->geturl(id(), fail_write_url_.c_str(), NULL);
+ return NPERR_NO_ERROR;
}
std::string url = SELF_URL;
@@ -107,6 +116,11 @@ NPError PluginGetURLTest::NewStream(NPMIMEType type, NPStream* stream,
return NPERR_NO_ERROR;
}
+ if (!fail_write_url_.empty()) {
+ return NPERR_NO_ERROR;
+ }
+
+
unsigned long stream_id = reinterpret_cast<unsigned long>(
stream->notifyData);
@@ -160,6 +174,11 @@ int32 PluginGetURLTest::Write(NPStream *stream, int32 offset, int32 len,
return PluginTest::Write(stream, offset, len, buffer);
}
+ if (!fail_write_url_.empty()) {
+ SignalTestCompleted();
+ return -1;
+ }
+
if (stream == NULL)
SetError("Write got null stream");
if (len < 0 || len > STREAM_CHUNK)
diff --git a/webkit/glue/plugins/test/plugin_geturl_test.h b/webkit/glue/plugins/test/plugin_geturl_test.h
index 1ff443b..6372ef9 100644
--- a/webkit/glue/plugins/test/plugin_geturl_test.h
+++ b/webkit/glue/plugins/test/plugin_geturl_test.h
@@ -46,6 +46,7 @@ class PluginGetURLTest : public PluginTest {
// This flag is set to true in the context of the NPN_Evaluate call.
bool npn_evaluate_context_;
std::string page_not_found_url_;
+ std::string fail_write_url_;
};
} // namespace NPAPIClient