- Revision
- 118722
- Author
- [email protected]
- Date
- 2012-05-28 21:11:05 -0700 (Mon, 28 May 2012)
Log Message
[V8] Avoid passing NULL to an 'isolate' parameter
https://bugs.webkit.org/show_bug.cgi?id=87689
Reviewed by Adam Barth.
v8::Null(isolate) crashes if we pass a NULL isolate.
Thus we are planning to replace v8::Null()s in a following way:
- Implement V8Bindings::v8Null(isolate). v8Null(isolate) does the NULL check.
If isolate is NULL, v8Null(isolate) calls v8::Null(). Otherwise,
v8Null(isolate) calls v8::Null(isolate).
- In V8 bindings, we replace v8::Null() with v8::Null(isolate) for a
non-optional 'isolate' parameter.
(e.g. void foo(..., Isolate* isolate) { v8::Null(); } )
- In V8 bindings, we replace v8::Null() with v8Null(isolate) for an
optional 'isolate' parameter.
(e.g. void foo(..., Isolate* isolate = 0) { v8::Null(); } )
However, currently we cannot do the replacement mechanically, since some code
pass NULL to a non-optional 'isolate' parameter. In other words, currently
"non-optional" does not guarantee that 'isolate' is not NULL.
This patch removes all the code that passes NULL to a non-optional 'isolate'
parameter. This will enable us to achieve the replacement mechanically.
No tests. No behavior change.
* bindings/scripts/CodeGeneratorV8.pm:
(GenerateCallbackImplementation):
(NativeToJSValue):
* bindings/scripts/test/V8/V8TestCallback.cpp:
(WebCore::V8TestCallback::callbackWithClass1Param):
(WebCore::V8TestCallback::callbackWithClass2Param):
(WebCore::V8TestCallback::callbackWithStringList):
(WebCore::V8TestCallback::callbackRequiresThisToPass):
* bindings/scripts/test/V8/V8TestObj.cpp:
(WebCore::V8TestObj::installPerContextProperties):
* bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp:
(WebCore::V8SQLStatementErrorCallback::handleEvent):
* bindings/v8/custom/V8MutationCallbackCustom.cpp:
(WebCore::V8MutationCallback::handleEvent):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (118721 => 118722)
--- trunk/Source/WebCore/ChangeLog 2012-05-29 04:08:42 UTC (rev 118721)
+++ trunk/Source/WebCore/ChangeLog 2012-05-29 04:11:05 UTC (rev 118722)
@@ -1,3 +1,49 @@
+2012-05-28 Kentaro Hara <[email protected]>
+
+ [V8] Avoid passing NULL to an 'isolate' parameter
+ https://bugs.webkit.org/show_bug.cgi?id=87689
+
+ Reviewed by Adam Barth.
+
+ v8::Null(isolate) crashes if we pass a NULL isolate.
+ Thus we are planning to replace v8::Null()s in a following way:
+
+ - Implement V8Bindings::v8Null(isolate). v8Null(isolate) does the NULL check.
+ If isolate is NULL, v8Null(isolate) calls v8::Null(). Otherwise,
+ v8Null(isolate) calls v8::Null(isolate).
+
+ - In V8 bindings, we replace v8::Null() with v8::Null(isolate) for a
+ non-optional 'isolate' parameter.
+ (e.g. void foo(..., Isolate* isolate) { v8::Null(); } )
+
+ - In V8 bindings, we replace v8::Null() with v8Null(isolate) for an
+ optional 'isolate' parameter.
+ (e.g. void foo(..., Isolate* isolate = 0) { v8::Null(); } )
+
+ However, currently we cannot do the replacement mechanically, since some code
+ pass NULL to a non-optional 'isolate' parameter. In other words, currently
+ "non-optional" does not guarantee that 'isolate' is not NULL.
+
+ This patch removes all the code that passes NULL to a non-optional 'isolate'
+ parameter. This will enable us to achieve the replacement mechanically.
+
+ No tests. No behavior change.
+
+ * bindings/scripts/CodeGeneratorV8.pm:
+ (GenerateCallbackImplementation):
+ (NativeToJSValue):
+ * bindings/scripts/test/V8/V8TestCallback.cpp:
+ (WebCore::V8TestCallback::callbackWithClass1Param):
+ (WebCore::V8TestCallback::callbackWithClass2Param):
+ (WebCore::V8TestCallback::callbackWithStringList):
+ (WebCore::V8TestCallback::callbackRequiresThisToPass):
+ * bindings/scripts/test/V8/V8TestObj.cpp:
+ (WebCore::V8TestObj::installPerContextProperties):
+ * bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp:
+ (WebCore::V8SQLStatementErrorCallback::handleEvent):
+ * bindings/v8/custom/V8MutationCallbackCustom.cpp:
+ (WebCore::V8MutationCallback::handleEvent):
+
2012-05-28 Kent Tamura <[email protected]>
Form controls in <fieldset disabled> should not be validated.
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm (118721 => 118722)
--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2012-05-29 04:08:42 UTC (rev 118721)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2012-05-29 04:11:05 UTC (rev 118722)
@@ -3098,7 +3098,7 @@
@args = ();
foreach my $param (@params) {
my $paramName = $param->name;
- push(@implContent, " v8::Handle<v8::Value> ${paramName}Handle = " . NativeToJSValue($param, $paramName, "0") . ";\n");
+ push(@implContent, " v8::Handle<v8::Value> ${paramName}Handle = " . NativeToJSValue($param, $paramName) . ";\n");
push(@implContent, " if (${paramName}Handle.IsEmpty()) {\n");
push(@implContent, " if (!isScriptControllerTerminating())\n");
push(@implContent, " CRASH();\n");
@@ -3798,6 +3798,7 @@
my $signature = shift;
my $value = shift;
my $getIsolate = shift;
+ $getIsolate = $getIsolate ? ", $getIsolate" : "";
my $type = GetTypeFromSignature($signature);
return "v8Boolean($value)" if $type eq "boolean";
@@ -3829,13 +3830,13 @@
if ($codeGenerator->IsStringType($type)) {
my $conv = $signature->extendedAttributes->{"TreatReturnedNullStringAs"};
if (defined $conv) {
- return "v8StringOrNull($value, $getIsolate)" if $conv eq "Null";
- return "v8StringOrUndefined($value, $getIsolate)" if $conv eq "Undefined";
- return "v8StringOrFalse($value, $getIsolate)" if $conv eq "False";
+ return "v8StringOrNull($value$getIsolate)" if $conv eq "Null";
+ return "v8StringOrUndefined($value$getIsolate)" if $conv eq "Undefined";
+ return "v8StringOrFalse($value$getIsolate)" if $conv eq "False";
die "Unknown value for TreatReturnedNullStringAs extended attribute";
}
- return "v8String($value, $getIsolate)";
+ return "v8String($value$getIsolate)";
}
my $arrayType = $codeGenerator->GetArrayType($type);
@@ -3844,18 +3845,18 @@
AddToImplIncludes("V8$arrayType.h");
AddToImplIncludes("$arrayType.h");
}
- return "v8Array($value, $getIsolate)";
+ return "v8Array($value$getIsolate)";
}
AddIncludesForType($type);
# special case for non-DOM node interfaces
if (IsDOMNodeType($type)) {
- return "toV8(${value}" . ($signature->extendedAttributes->{"ReturnNewObject"} ? ", $getIsolate, true)" : ", $getIsolate)");
+ return "toV8(${value}" . ($signature->extendedAttributes->{"ReturnNewObject"} ? "$getIsolate, true)" : "$getIsolate)");
}
if ($type eq "EventTarget") {
- return "V8DOMWrapper::convertEventTargetToV8Object($value, $getIsolate)";
+ return "V8DOMWrapper::convertEventTargetToV8Object($value$getIsolate)";
}
if ($type eq "EventListener") {
@@ -3872,7 +3873,7 @@
AddToImplIncludes("wtf/RefPtr.h");
AddToImplIncludes("wtf/GetPtr.h");
- return "toV8($value, $getIsolate)";
+ return "toV8($value$getIsolate)";
}
sub ReturnNativeToJSValue
Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestCallback.cpp (118721 => 118722)
--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestCallback.cpp 2012-05-29 04:08:42 UTC (rev 118721)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestCallback.cpp 2012-05-29 04:11:05 UTC (rev 118722)
@@ -87,7 +87,7 @@
v8::Context::Scope scope(v8Context);
- v8::Handle<v8::Value> class1ParamHandle = toV8(class1Param, 0);
+ v8::Handle<v8::Value> class1ParamHandle = toV8(class1Param);
if (class1ParamHandle.IsEmpty()) {
if (!isScriptControllerTerminating())
CRASH();
@@ -115,13 +115,13 @@
v8::Context::Scope scope(v8Context);
- v8::Handle<v8::Value> class2ParamHandle = toV8(class2Param, 0);
+ v8::Handle<v8::Value> class2ParamHandle = toV8(class2Param);
if (class2ParamHandle.IsEmpty()) {
if (!isScriptControllerTerminating())
CRASH();
return true;
}
- v8::Handle<v8::Value> strArgHandle = v8String(strArg, 0);
+ v8::Handle<v8::Value> strArgHandle = v8String(strArg);
if (strArgHandle.IsEmpty()) {
if (!isScriptControllerTerminating())
CRASH();
@@ -150,7 +150,7 @@
v8::Context::Scope scope(v8Context);
- v8::Handle<v8::Value> listParamHandle = toV8(listParam, 0);
+ v8::Handle<v8::Value> listParamHandle = toV8(listParam);
if (listParamHandle.IsEmpty()) {
if (!isScriptControllerTerminating())
CRASH();
@@ -208,13 +208,13 @@
v8::Context::Scope scope(v8Context);
- v8::Handle<v8::Value> class8ParamHandle = toV8(class8Param, 0);
+ v8::Handle<v8::Value> class8ParamHandle = toV8(class8Param);
if (class8ParamHandle.IsEmpty()) {
if (!isScriptControllerTerminating())
CRASH();
return true;
}
- v8::Handle<v8::Value> thisClassParamHandle = toV8(thisClassParam, 0);
+ v8::Handle<v8::Value> thisClassParamHandle = toV8(thisClassParam);
if (thisClassParamHandle.IsEmpty()) {
if (!isScriptControllerTerminating())
CRASH();
Modified: trunk/Source/WebCore/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp (118721 => 118722)
--- trunk/Source/WebCore/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp 2012-05-29 04:08:42 UTC (rev 118721)
+++ trunk/Source/WebCore/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp 2012-05-29 04:11:05 UTC (rev 118722)
@@ -56,8 +56,8 @@
v8::Context::Scope scope(v8Context);
- v8::Handle<v8::Value> transactionHandle = toV8(transaction, 0);
- v8::Handle<v8::Value> errorHandle = toV8(error, 0);
+ v8::Handle<v8::Value> transactionHandle = toV8(transaction);
+ v8::Handle<v8::Value> errorHandle = toV8(error);
if (transactionHandle.IsEmpty() || errorHandle.IsEmpty()) {
if (!isScriptControllerTerminating())
CRASH();
Modified: trunk/Source/WebCore/bindings/v8/custom/V8MutationCallbackCustom.cpp (118721 => 118722)
--- trunk/Source/WebCore/bindings/v8/custom/V8MutationCallbackCustom.cpp 2012-05-29 04:08:42 UTC (rev 118721)
+++ trunk/Source/WebCore/bindings/v8/custom/V8MutationCallbackCustom.cpp 2012-05-29 04:11:05 UTC (rev 118722)
@@ -66,9 +66,9 @@
v8::Local<v8::Array> mutationsArray = v8::Array::New(mutations->size());
for (size_t i = 0; i < mutations->size(); ++i)
- mutationsArray->Set(v8::Integer::New(i), toV8(mutations->at(i).get(), 0));
+ mutationsArray->Set(v8::Integer::New(i), toV8(mutations->at(i).get()));
- v8::Handle<v8::Value> observerHandle = toV8(observer, 0);
+ v8::Handle<v8::Value> observerHandle = toV8(observer);
if (observerHandle.IsEmpty()) {
if (!isScriptControllerTerminating())
CRASH();
