From ead9b4508c4b1199a2eebbda86743d46d7a1ab5d Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Mon, 21 Nov 2011 19:01:22 +0000 Subject: [PATCH] - Allow registration of V8 extensions with no native function handler (issue #433). - Add a CefV8Context::InContext() method to test if V8 is currently in a context (issue #427). - Verify that a current context exists when creating V8 arrays, functions and objects (issue #427). - Add a v8::HandleScope in GetCurrentContext() and GetEnteredContext() to avoid "Cannot create a handle without a HandleScope" V8 errors (issue #427). git-svn-id: https://chromiumembedded.googlecode.com/svn/trunk@391 5089003a-bbd8-11dd-ad1f-f1f9622dbc98 --- include/cef.h | 6 ++ include/cef_capi.h | 5 ++ libcef/v8_impl.cc | 57 ++++++++++---- libcef_dll/cpptoc/v8context_cpptoc.cc | 5 ++ libcef_dll/ctocpp/v8context_ctocpp.cc | 5 ++ tests/unittests/v8_unittest.cc | 102 ++++++++++++++++++++++++++ 6 files changed, 167 insertions(+), 13 deletions(-) diff --git a/include/cef.h b/include/cef.h index 296ff4f72..9a50e6189 100644 --- a/include/cef.h +++ b/include/cef.h @@ -2264,6 +2264,12 @@ public: /*--cef()--*/ static CefRefPtr GetEnteredContext(); + /// + // Returns true if V8 is currently inside a context. + /// + /*--cef()--*/ + static bool InContext(); + /// // Returns the browser for this context. /// diff --git a/include/cef_capi.h b/include/cef_capi.h index c7ce5bf66..91913f7fd 100644 --- a/include/cef_capi.h +++ b/include/cef_capi.h @@ -2081,6 +2081,11 @@ CEF_EXPORT cef_v8context_t* cef_v8context_get_current_context(); /// CEF_EXPORT cef_v8context_t* cef_v8context_get_entered_context(); +/// +// Returns true (1) if V8 is currently inside a context. +/// +CEF_EXPORT int cef_v8context_in_context(); + /// // Structure that should be implemented to handle V8 function calls. The diff --git a/libcef/v8_impl.cc b/libcef/v8_impl.cc index 732fd6698..8849af6c4 100644 --- a/libcef/v8_impl.cc +++ b/libcef/v8_impl.cc @@ -259,13 +259,18 @@ public: CefV8Handler* handler) : v8::Extension(extension_name, javascript_code), handler_(handler) { - // The reference will be released when the application exits. - TrackAdd(new TrackBase(handler)); + if (handler) { + // The reference will be released when the application exits. + TrackAdd(new TrackBase(handler)); + } } virtual v8::Handle GetNativeFunction( v8::Handle name) { + if (!handler_) + return v8::Handle(); + return v8::FunctionTemplate::New(FunctionCallbackImpl, v8::External::Wrap(handler_)); } @@ -341,11 +346,6 @@ bool CefRegisterExtension(const CefString& extension_name, // Verify that the context is in a valid state. CEF_REQUIRE_VALID_CONTEXT(false); - if(!handler.get()) { - NOTREACHED(); - return false; - } - TrackString* name = new TrackString(extension_name); TrackAdd(name); TrackString* code = new TrackString(javascript_code); @@ -368,8 +368,10 @@ CefRefPtr CefV8Context::GetCurrentContext() CefRefPtr context; CEF_REQUIRE_VALID_CONTEXT(context); CEF_REQUIRE_UI_THREAD(context); - if (v8::Context::InContext()) - context = new CefV8ContextImpl( v8::Context::GetCurrent() ); + if (v8::Context::InContext()) { + v8::HandleScope handle_scope; + context = new CefV8ContextImpl(v8::Context::GetCurrent()); + } return context; } @@ -379,11 +381,21 @@ CefRefPtr CefV8Context::GetEnteredContext() CefRefPtr context; CEF_REQUIRE_VALID_CONTEXT(context); CEF_REQUIRE_UI_THREAD(context); - if (v8::Context::InContext()) - context = new CefV8ContextImpl( v8::Context::GetEntered() ); + if (v8::Context::InContext()) { + v8::HandleScope handle_scope; + context = new CefV8ContextImpl(v8::Context::GetEntered()); + } return context; } +// static +bool CefV8Context::InContext() +{ + CEF_REQUIRE_VALID_CONTEXT(false); + CEF_REQUIRE_UI_THREAD(false); + return v8::Context::InContext(); +} + // CefV8ContextImpl @@ -571,6 +583,12 @@ CefRefPtr CefV8Value::CreateObject( v8::HandleScope handle_scope; + v8::Local context = v8::Context::GetCurrent(); + if (context.IsEmpty()) { + NOTREACHED() << "not currently in a V8 context"; + return NULL; + } + // Create the new V8 object. v8::Local obj = v8::Object::New(); @@ -606,6 +624,13 @@ CefRefPtr CefV8Value::CreateArray() CEF_REQUIRE_UI_THREAD(NULL); v8::HandleScope handle_scope; + + v8::Local context = v8::Context::GetCurrent(); + if (context.IsEmpty()) { + NOTREACHED() << "not currently in a V8 context"; + return NULL; + } + return new CefV8ValueImpl(v8::Array::New()); } @@ -622,7 +647,13 @@ CefRefPtr CefV8Value::CreateFunction(const CefString& name, } v8::HandleScope handle_scope; - + + v8::Local context = v8::Context::GetCurrent(); + if (context.IsEmpty()) { + NOTREACHED() << "not currently in a V8 context"; + return NULL; + } + // Create a new V8 function template with one internal field. v8::Local tmpl = v8::FunctionTemplate::New(); @@ -634,7 +665,7 @@ CefRefPtr CefV8Value::CreateFunction(const CefString& name, // Retrieve the function object and set the name. v8::Local func = tmpl->GetFunction(); if (func.IsEmpty()) { - NOTREACHED() << "failed to create function"; + NOTREACHED() << "failed to create V8 function"; return NULL; } diff --git a/libcef_dll/cpptoc/v8context_cpptoc.cc b/libcef_dll/cpptoc/v8context_cpptoc.cc index 79889371f..93f75d86a 100644 --- a/libcef_dll/cpptoc/v8context_cpptoc.cc +++ b/libcef_dll/cpptoc/v8context_cpptoc.cc @@ -36,6 +36,11 @@ CEF_EXPORT cef_v8context_t* cef_v8context_get_entered_context() return NULL; } +CEF_EXPORT int cef_v8context_in_context() +{ + return CefV8Context::InContext(); +} + // MEMBER FUNCTIONS - Body may be edited by hand. diff --git a/libcef_dll/ctocpp/v8context_ctocpp.cc b/libcef_dll/ctocpp/v8context_ctocpp.cc index 371e4c4ac..ad63bfeac 100644 --- a/libcef_dll/ctocpp/v8context_ctocpp.cc +++ b/libcef_dll/ctocpp/v8context_ctocpp.cc @@ -34,6 +34,11 @@ CefRefPtr CefV8Context::GetEnteredContext() return NULL; } +bool CefV8Context::InContext() +{ + return cef_v8context_in_context() ? true : false; +} + // VIRTUAL METHODS - Body may be edited by hand. diff --git a/tests/unittests/v8_unittest.cc b/tests/unittests/v8_unittest.cc index 068f3ffa9..eda4bc794 100644 --- a/tests/unittests/v8_unittest.cc +++ b/tests/unittests/v8_unittest.cc @@ -391,6 +391,108 @@ TEST(V8Test, Extension) namespace { +class TestNoNativeHandler : public TestHandler +{ +public: + class TestHandler : public CefV8Handler + { + public: + TestHandler(CefRefPtr test) + : test_(test) + { + } + + virtual bool Execute(const CefString& name, + CefRefPtr object, + const CefV8ValueList& arguments, + CefRefPtr& retval, + CefString& exception) OVERRIDE + { + if (name == "result") { + if (arguments.size() == 1 && arguments[0]->IsString()) { + std::string value = arguments[0]->GetStringValue(); + if (value == "correct") + test_->got_correct_.yes(); + else + return false; + return true; + } + } + + return false; + } + + CefRefPtr test_; + + IMPLEMENT_REFCOUNTING(TestHandler); + }; + + TestNoNativeHandler() + { + } + + virtual void RunTest() OVERRIDE + { + std::string testHtml = + "\n" + "\n" + ""; + AddResource("http://tests/run.html", testHtml, "text/html"); + + CreateBrowser("http://tests/run.html"); + } + + virtual void OnLoadEnd(CefRefPtr browser, + CefRefPtr frame, + int httpStatusCode) OVERRIDE + { + DestroyTest(); + } + + virtual void OnJSBinding(CefRefPtr browser, + CefRefPtr frame, + CefRefPtr object) OVERRIDE + { + // Create the functions that will be used during the test. + CefRefPtr obj = CefV8Value::CreateObject(NULL, NULL); + CefRefPtr handler = new TestHandler(this); + obj->SetValue("result", + CefV8Value::CreateFunction("result", handler), + V8_PROPERTY_ATTRIBUTE_NONE); + object->SetValue("test", obj, V8_PROPERTY_ATTRIBUTE_NONE); + } + + TrackCallback got_correct_; +}; + +} // namespace + +// Verify extensions with no native functions +TEST(V8Test, ExtensionNoNative) +{ + std::string extensionCode = + "var test_nonative;" + "if (!test_nonative)" + " test_nonative = {};" + "(function() {" + " test_nonative.add = function(a, b) {" + " return a + b;" + " };" + "})();"; + CefRegisterExtension("v8/test_nonative", extensionCode, NULL); + + CefRefPtr handler = new TestNoNativeHandler(); + handler->ExecuteTest(); + + EXPECT_TRUE(handler->got_correct_); +} + +namespace { + // Using a delegate so that the code below can remain inline. class CefV8HandlerDelegate {