Diff
Modified: trunk/LayoutTests/ChangeLog (115357 => 115358)
--- trunk/LayoutTests/ChangeLog 2012-04-26 21:05:16 UTC (rev 115357)
+++ trunk/LayoutTests/ChangeLog 2012-04-26 21:07:56 UTC (rev 115358)
@@ -1,3 +1,18 @@
+2012-04-26 Benjamin Poulain <[email protected]>
+
+ ObjcClass::methodsNamed() can leak if buffer is dynamically allocated
+ https://bugs.webkit.org/show_bug.cgi?id=84668
+
+ Reviewed by Alexey Proskuryakov.
+
+ Extend the test coverage to accessor with very long name. Add coverage for the
+ _javascript_ to Objective-C method name conversion.
+
+ * platform/mac/plugins/bindings-objc-long-method-name-expected.txt: Added.
+ * platform/mac/plugins/bindings-objc-long-method-name.html: Added.
+ * platform/mac/plugins/bindings-objc-method-name-conversion-expected.txt: Added.
+ * platform/mac/plugins/bindings-objc-method-name-conversion.html: Added.
+
2012-04-26 Dimitri Glazkov <[email protected]>
[Chromium] media/video-currentTime-set.html is flaky
Added: trunk/LayoutTests/platform/mac/plugins/bindings-objc-long-method-name-expected.txt (0 => 115358)
--- trunk/LayoutTests/platform/mac/plugins/bindings-objc-long-method-name-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/mac/plugins/bindings-objc-long-method-name-expected.txt 2012-04-26 21:07:56 UTC (rev 115358)
@@ -0,0 +1,10 @@
+Test invoking a method with a long name does not cause crash or memory leak.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS objCPlugin[methodName]() is "methodMappedToLongName"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/platform/mac/plugins/bindings-objc-long-method-name.html (0 => 115358)
--- trunk/LayoutTests/platform/mac/plugins/bindings-objc-long-method-name.html (rev 0)
+++ trunk/LayoutTests/platform/mac/plugins/bindings-objc-long-method-name.html 2012-04-26 21:07:56 UTC (rev 115358)
@@ -0,0 +1,30 @@
+<html>
+<head>
+<script src=""
+<script>
+var methodName = '';
+function doTest()
+{
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+ for (var i = 0; i < 1024; ++i)
+ methodName += 'long';
+
+ objCPlugin[methodName]();
+ for (var i = 0; i < 1000; ++i)
+ objCPlugin[methodName]();
+ shouldBeEqualToString('objCPlugin[methodName]()', 'methodMappedToLongName');
+ finishJSTest();
+}
+</script>
+</head>
+<body _onload_="doTest();">
+<embed id="testCPlugin" type="application/x-webkit-test-netscape"></embed>
+<script>
+ description("Test invoking a method with a long name does not cause crash or memory leak.");
+ jsTestIsAsync=true;
+</script>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/platform/mac/plugins/bindings-objc-method-name-conversion-expected.txt (0 => 115358)
--- trunk/LayoutTests/platform/mac/plugins/bindings-objc-method-name-conversion-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/mac/plugins/bindings-objc-method-name-conversion-expected.txt 2012-04-26 21:07:56 UTC (rev 115358)
@@ -0,0 +1,15 @@
+Test the name conversion when invoking Objective-C from _javascript_.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS objCPlugin['testConversionColon_'](1) is "testConversionColon:(int)useless"
+PASS objCPlugin['testConversionEscapeChar$$a$_b$$$__'](2) is "testConversionEscapeChar$a_b$_:(int)useless"
+PASS objCPlugin['tes$tCo$nver$sion$Col$on_'](1) is "testConversionColon:(int)useless"
+PASS objCPlugin['testC$onv$ersi$onEs$capeC$har$$a$_b$$$__'](2) is "testConversionEscapeChar$a_b$_:(int)useless"
+PASS objCPlugin['testConversionColon_$'](1) is "testConversionColon:(int)useless"
+PASS objCPlugin['testConversionEscapeChar$$a$_b$$$__$'](2) is "testConversionEscapeChar$a_b$_:(int)useless"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/platform/mac/plugins/bindings-objc-method-name-conversion.html (0 => 115358)
--- trunk/LayoutTests/platform/mac/plugins/bindings-objc-method-name-conversion.html (rev 0)
+++ trunk/LayoutTests/platform/mac/plugins/bindings-objc-method-name-conversion.html 2012-04-26 21:07:56 UTC (rev 115358)
@@ -0,0 +1,34 @@
+<html>
+<head>
+<script src=""
+<script>
+var methodName = '';
+function doTest()
+{
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+ shouldBeEqualToString("objCPlugin['testConversionColon_'](1)", 'testConversionColon:(int)useless');
+ shouldBeEqualToString("objCPlugin['testConversionEscapeChar$$a$_b$$$__'](2)", 'testConversionEscapeChar$a_b$_:(int)useless');
+
+ // "$" alone is just ignored by the conversion.
+ shouldBeEqualToString("objCPlugin['tes$tCo$nver$sion$Col$on_'](1)", 'testConversionColon:(int)useless');
+ shouldBeEqualToString("objCPlugin['testC$onv$ersi$onEs$capeC$har$$a$_b$$$__'](2)", 'testConversionEscapeChar$a_b$_:(int)useless');
+
+ // Since "$" is skipped. Test a trailing dollar does not overflow the input.
+ shouldBeEqualToString("objCPlugin['testConversionColon_$'](1)", 'testConversionColon:(int)useless');
+ shouldBeEqualToString("objCPlugin['testConversionEscapeChar$$a$_b$$$__$'](2)", 'testConversionEscapeChar$a_b$_:(int)useless');
+
+ finishJSTest();
+}
+</script>
+</head>
+<body _onload_="doTest();">
+<embed id="testCPlugin" type="application/x-webkit-test-netscape"></embed>
+<script>
+ description("Test the name conversion when invoking Objective-C from _javascript_.");
+ jsTestIsAsync=true;
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (115357 => 115358)
--- trunk/Source/WebCore/ChangeLog 2012-04-26 21:05:16 UTC (rev 115357)
+++ trunk/Source/WebCore/ChangeLog 2012-04-26 21:07:56 UTC (rev 115358)
@@ -1,3 +1,21 @@
+2012-04-26 Benjamin Poulain <[email protected]>
+
+ ObjcClass::methodsNamed() can leak if buffer is dynamically allocated
+ https://bugs.webkit.org/show_bug.cgi?id=84668
+
+ Reviewed by Alexey Proskuryakov.
+
+ Change ObjcClass::methodsNamed() to be based on a vector instead of managing
+ the memory manually.
+
+ Tests: platform/mac/plugins/bindings-objc-long-method-name.html
+ platform/mac/plugins/bindings-objc-method-name-conversion.html
+
+ * bridge/objc/objc_class.mm:
+ (Bindings):
+ (JSC::Bindings::convertJSMethodNameToObjc):
+ (JSC::Bindings::ObjcClass::methodsNamed):
+
2012-04-26 Justin Novosad <[email protected]>
[Chromium] Single buffered canvas layers with the threaded compositor
Modified: trunk/Source/WebCore/bridge/objc/objc_class.mm (115357 => 115358)
--- trunk/Source/WebCore/bridge/objc/objc_class.mm 2012-04-26 21:05:16 UTC (rev 115357)
+++ trunk/Source/WebCore/bridge/objc/objc_class.mm 2012-04-26 21:07:56 UTC (rev 115358)
@@ -80,57 +80,37 @@
This function performs the inverse of that operation.
@result Fills 'buffer' with the ObjectiveC method name that corresponds to 'JSName'.
- Returns true for success, false for failure. (Failure occurs when 'buffer'
- is not big enough to hold the result.)
*/
-static bool convertJSMethodNameToObjc(const char *JSName, char *buffer, size_t bufferSize)
+typedef Vector<char, 256> JSNameConversionBuffer;
+static inline void convertJSMethodNameToObjc(const CString& jsName, JSNameConversionBuffer& buffer)
{
- ASSERT(JSName && buffer);
+ buffer.reserveInitialCapacity(jsName.length() + 1);
- const char *sp = JSName; // source pointer
- char *dp = buffer; // destination pointer
-
- char *end = buffer + bufferSize;
- while (dp < end) {
- if (*sp == '$') {
- ++sp;
- *dp = *sp;
- } else if (*sp == '_')
- *dp = ':';
+ const char* source = jsName.data();
+ while (true) {
+ if (*source == '$') {
+ ++source;
+ buffer.uncheckedAppend(*source);
+ } else if (*source == '_')
+ buffer.uncheckedAppend(':');
else
- *dp = *sp;
+ buffer.uncheckedAppend(*source);
- // If a future coder puts funny ++ operators above, we might write off the end
- // of the buffer in the middle of this loop. Let's make sure to check for that.
- ASSERT(dp < end);
+ if (!*source)
+ return;
- if (*sp == 0) { // We finished converting JSName
- ASSERT(strlen(JSName) < bufferSize);
- return true;
- }
-
- ++sp;
- ++dp;
+ ++source;
}
-
- return false; // We ran out of buffer before converting JSName
}
MethodList ObjcClass::methodsNamed(const Identifier& identifier, Instance*) const
{
- MethodList methodList;
- char fixedSizeBuffer[1024];
- char* buffer = fixedSizeBuffer;
CString jsName = identifier.ascii();
- if (!convertJSMethodNameToObjc(jsName.data(), buffer, sizeof(fixedSizeBuffer))) {
- int length = jsName.length() + 1;
- buffer = new char[length];
- if (!buffer || !convertJSMethodNameToObjc(jsName.data(), buffer, length))
- return methodList;
- }
+ JSNameConversionBuffer buffer;
+ convertJSMethodNameToObjc(jsName, buffer);
-
- RetainPtr<CFStringRef> methodName(AdoptCF, CFStringCreateWithCString(NULL, buffer, kCFStringEncodingASCII));
+ MethodList methodList;
+ RetainPtr<CFStringRef> methodName(AdoptCF, CFStringCreateWithCString(NULL, buffer.data(), kCFStringEncodingASCII));
Method* method = (Method*)CFDictionaryGetValue(_methods.get(), methodName.get());
if (method) {
methodList.append(method);
@@ -158,7 +138,7 @@
if ([thisClass respondsToSelector:@selector(webScriptNameForSelector:)])
mappedName = [thisClass webScriptNameForSelector:objcMethodSelector];
- if ((mappedName && [mappedName isEqual:(NSString*)methodName.get()]) || strcmp(objcMethodSelectorName, buffer) == 0) {
+ if ((mappedName && [mappedName isEqual:(NSString*)methodName.get()]) || strcmp(objcMethodSelectorName, buffer.data()) == 0) {
Method* aMethod = new ObjcMethod(thisClass, objcMethodSelector); // deleted when the dictionary is destroyed
CFDictionaryAddValue(_methods.get(), methodName.get(), aMethod);
methodList.append(aMethod);
@@ -169,9 +149,6 @@
free(objcMethodList);
}
- if (buffer != fixedSizeBuffer)
- delete [] buffer;
-
return methodList;
}
Modified: trunk/Tools/ChangeLog (115357 => 115358)
--- trunk/Tools/ChangeLog 2012-04-26 21:05:16 UTC (rev 115357)
+++ trunk/Tools/ChangeLog 2012-04-26 21:07:56 UTC (rev 115358)
@@ -1,3 +1,19 @@
+2012-04-26 Benjamin Poulain <[email protected]>
+
+ ObjcClass::methodsNamed() can leak if buffer is dynamically allocated
+ https://bugs.webkit.org/show_bug.cgi?id=84668
+
+ Reviewed by Alexey Proskuryakov.
+
+ Extend ObjCPlugin to support the new layout tests of the Objective-C bridge.
+
+ * DumpRenderTree/mac/ObjCPlugin.m:
+ (+[ObjCPlugin isSelectorExcludedFromWebScript:]):
+ (+[ObjCPlugin webScriptNameForSelector:]):
+ (-[ObjCPlugin methodMappedToLongName]):
+ (-[ObjCPlugin testConversionColon:]):
+ (-[ObjCPlugin _:]):
+
2012-04-26 Dimitri Glazkov <[email protected]>
Unreviewed, rolling out r115340.
Modified: trunk/Tools/DumpRenderTree/mac/ObjCPlugin.m (115357 => 115358)
--- trunk/Tools/DumpRenderTree/mac/ObjCPlugin.m 2012-04-26 21:05:16 UTC (rev 115357)
+++ trunk/Tools/DumpRenderTree/mac/ObjCPlugin.m 2012-04-26 21:07:56 UTC (rev 115358)
@@ -142,8 +142,15 @@
return NO;
if (aSelector == @selector(throwIfArgumentIsNotHello:))
- return NO;
+ return NO;
+ if (aSelector == @selector(methodMappedToLongName))
+ return NO;
+
+ NSString *selectorName = NSStringFromSelector(aSelector);
+ if ([selectorName hasPrefix:@"testConversion"])
+ return NO;
+
return YES;
}
@@ -153,8 +160,11 @@
return @"echo";
if (aSelector == @selector(throwIfArgumentIsNotHello:))
- return @"throwIfArgumentIsNotHello";
+ return @"throwIfArgumentIsNotHello";
+ if (aSelector == @selector(methodMappedToLongName))
+ return [@"" stringByPaddingToLength:4096 withString: @"long" startingAtIndex:0];
+
return nil;
}
@@ -196,6 +206,21 @@
[WebScriptObject throwException:[NSString stringWithFormat:@"%@ != Hello", str]];
}
+- (NSString *)methodMappedToLongName
+{
+ return @"methodMappedToLongName";
+}
+
+- (NSString *)testConversionColon:(int)useless
+{
+ return @"testConversionColon:(int)useless";
+}
+
+- (NSString *)testConversionEscapeChar$a_b$_:(int)useless
+{
+ return @"testConversionEscapeChar$a_b$_:(int)useless";
+}
+
- (void)dealloc
{
if (throwOnDealloc)
