[webkit-reviews] review granted: [Bug 35401] Unsafe casting of Method types in bridge code. : [Attachment 49603] New awesomier patch.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Feb 26 12:16:15 PST 2010
Alexey Proskuryakov <ap at webkit.org> has granted Gavin Barraclough
<barraclough at apple.com>'s request for review:
Bug 35401: Unsafe casting of Method types in bridge code.
https://bugs.webkit.org/show_bug.cgi?id=35401
Attachment 49603: New awesomier patch.
https://bugs.webkit.org/attachment.cgi?id=49603&action=review
------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
__ZN7WebCore6String8fromUTF8EPKcm
+__ZTVN3JSC13RuntimeMethodE
__ZNK3JSC8Bindings13RuntimeObject12defaultValueEPNS_9ExecStateENS_22PreferredPr
imitiveTypeE
This doesn't look sorted properly.
+ virtual const ClassInfo* classInfo() const { return &info; }
In RuntimeMethod, it's s_info. Why does this work (or what is broken)?
+class CRuntimeMethod : public RuntimeMethod {
I didn't notice it last time, but we really prefer to have classes in their own
files in new code.
+#import "JavaScriptCore/Error.h"
#import "NetscapePluginHostProxy.h"
#import "ProxyRuntimeObject.h"
#import <WebCore/IdentifierRep.h>
#import <WebCore/JSDOMWindow.h>
#import <WebCore/npruntime_impl.h>
#import <runtime/PropertyNameArray.h>
+#import "WebCore/runtime_method.h"
This looks like a mess. I think it should be "runtime/Error.h" and
<WebCore/runtime_method.h>.
+ * java/java-and-plugins.html: Add tests for passing mismatched this
objects to methdods.
The patch and ChangeLog don't include changes for expected results.
With the exception of "one class per file", the comments above clearly must be
addressed before landing, r=me assuming you make the appropriate changes, and
also fix Qt build.
More information about the webkit-reviews
mailing list