diff options
| author | Devlin Cronin <rdevlin.cronin@chromium.org> | 2015-11-23 16:16:54 -0800 |
|---|---|---|
| committer | Devlin Cronin <rdevlin.cronin@chromium.org> | 2015-11-24 00:18:12 +0000 |
| commit | 9ab386229479ade5538db5a224dbae3998983240 (patch) | |
| tree | b3234acef0d7fa5346f56604409f86befe9d1888 | |
| parent | a08cc3736cdee241094fce1969f5d9b010f2c507 (diff) | |
| download | chromium_src-9ab386229479ade5538db5a224dbae3998983240.zip chromium_src-9ab386229479ade5538db5a224dbae3998983240.tar.gz chromium_src-9ab386229479ade5538db5a224dbae3998983240.tar.bz2 | |
[Extensions] Make handler_function a hidden property
Fix the glitch and add a test.
BUG=548273
Review URL: https://codereview.chromium.org/1422383003
Cr-Commit-Position: refs/heads/master@{#356985}
(cherry picked from commit a5ecbc86a598e29ce3a2424c9c5386bfba4e9b74)
Review URL: https://codereview.chromium.org/1469933003 .
Cr-Commit-Position: refs/branch-heads/2526@{#465}
Cr-Branched-From: cb947c0153db0ec02a8abbcb3ca086d88bf6006f-refs/heads/master@{#352221}
3 files changed, 43 insertions, 5 deletions
diff --git a/chrome/browser/extensions/extension_bindings_apitest.cc b/chrome/browser/extensions/extension_bindings_apitest.cc index 9843e7f..ed472ac 100644 --- a/chrome/browser/extensions/extension_bindings_apitest.cc +++ b/chrome/browser/extensions/extension_bindings_apitest.cc @@ -111,5 +111,24 @@ IN_PROC_BROWSER_TEST_F(ExtensionBindingsApiTest, ModuleSystem) { ASSERT_TRUE(RunExtensionTest("bindings/module_system")) << message_; } +IN_PROC_BROWSER_TEST_F(ExtensionBindingsApiTest, HandlerFunctionTypeChecking) { + ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady()); + ui_test_utils::NavigateToURL( + browser(), + embedded_test_server()->GetURL( + "/extensions/api_test/bindings/handler_function_type_checking.html")); + content::WebContents* web_contents = + browser()->tab_strip_model()->GetActiveWebContents(); + EXPECT_FALSE(web_contents->IsCrashed()); + // See handler_function_type_checking.html. + std::string result; + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + web_contents, + "window.domAutomationController.send(" + "document.getElementById('status').textContent.trim());", + &result)); + EXPECT_EQ("success", result); +} + } // namespace } // namespace extensions diff --git a/chrome/test/data/extensions/api_test/bindings/handler_function_type_checking.html b/chrome/test/data/extensions/api_test/bindings/handler_function_type_checking.html new file mode 100644 index 0000000..4d21d9f --- /dev/null +++ b/chrome/test/data/extensions/api_test/bindings/handler_function_type_checking.html @@ -0,0 +1,15 @@ +<!doctype html> +<body> +<span id="status">success</span> +<script> +iframe = document.body.appendChild(document.createElement("iframe")); +obj = iframe.contentWindow.Object; +app = iframe.contentWindow.chrome.app; +iframe.remove(); +obj.prototype.__defineGetter__("handler_function", function() { + document.getElementById('status').textContent = 'FAILED'; + return 0xBADDEAD; +}); +app.getDetails(); +</script> +</body> diff --git a/extensions/renderer/object_backed_native_handler.cc b/extensions/renderer/object_backed_native_handler.cc index 060606b..c27e63d 100644 --- a/extensions/renderer/object_backed_native_handler.cc +++ b/extensions/renderer/object_backed_native_handler.cc @@ -41,7 +41,8 @@ void ObjectBackedNativeHandler::Router( v8::Local<v8::Object> data = args.Data().As<v8::Object>(); v8::Local<v8::Value> handler_function_value = - data->Get(v8::String::NewFromUtf8(args.GetIsolate(), kHandlerFunction)); + data->GetHiddenValue( + v8::String::NewFromUtf8(args.GetIsolate(), kHandlerFunction)); // See comment in header file for why we do this. if (handler_function_value.IsEmpty() || handler_function_value->IsUndefined()) { @@ -51,7 +52,9 @@ void ObjectBackedNativeHandler::Router( "Extension view no longer exists"); return; } - DCHECK(handler_function_value->IsExternal()); + // This CHECK is *important*. Otherwise, we'll go around happily executing + // something random. See crbug.com/548273. + CHECK(handler_function_value->IsExternal()); static_cast<HandlerFunction*>( handler_function_value.As<v8::External>()->Value())->Run(args); } @@ -64,7 +67,7 @@ void ObjectBackedNativeHandler::RouteFunction( v8::Context::Scope context_scope(context_->v8_context()); v8::Local<v8::Object> data = v8::Object::New(isolate); - data->Set( + data->SetHiddenValue( v8::String::NewFromUtf8(isolate, kHandlerFunction), v8::External::New(isolate, new HandlerFunction(handler_function))); v8::Local<v8::FunctionTemplate> function_template = @@ -86,11 +89,12 @@ void ObjectBackedNativeHandler::Invalidate() { for (size_t i = 0; i < router_data_.Size(); i++) { v8::Local<v8::Object> data = router_data_.Get(i); v8::Local<v8::Value> handler_function_value = - data->Get(v8::String::NewFromUtf8(isolate, kHandlerFunction)); + data->GetHiddenValue( + v8::String::NewFromUtf8(isolate, kHandlerFunction)); CHECK(!handler_function_value.IsEmpty()); delete static_cast<HandlerFunction*>( handler_function_value.As<v8::External>()->Value()); - data->Delete(v8::String::NewFromUtf8(isolate, kHandlerFunction)); + data->DeleteHiddenValue(v8::String::NewFromUtf8(isolate, kHandlerFunction)); } router_data_.Clear(); |