To kill V8Proxy, we can move compileScript() from V8Proxy to ScriptController.
Created attachment 159589 [details] Patch
Comment on attachment 159589 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159589&action=review > Source/WebCore/bindings/v8/ScriptController.cpp:342 > + const uint16_t* fileNameString = fromWebCoreString(fileName); What is fromWebCoreString ? > Source/WebCore/bindings/v8/ScriptController.h:91 > + static v8::Handle<v8::Script> compileScript(v8::Handle<v8::String> code, const String& fileName, const TextPosition& scriptStartPosition, v8::ScriptData* = 0); Should this really go on script controller? I guess I would have expected this to be related to ScriptSourceCode.... I guess the types don't quite line up.
(In reply to comment #2) > > Source/WebCore/bindings/v8/ScriptController.cpp:342 > > + const uint16_t* fileNameString = fromWebCoreString(fileName); > > What is fromWebCoreString ? fromWebCoreString() retrieves uint16 from WebCore String. Maybe should we rename it to uint16OfWebCoreString()? > > Source/WebCore/bindings/v8/ScriptController.h:91 > > + static v8::Handle<v8::Script> compileScript(v8::Handle<v8::String> code, const String& fileName, const TextPosition& scriptStartPosition, v8::ScriptData* = 0); > > Should this really go on script controller? I guess I would have expected this to be related to ScriptSourceCode.... I guess the types don't quite line up. Makes sense. I'll update the patch.
(In reply to comment #3) > (In reply to comment #2) > > > Source/WebCore/bindings/v8/ScriptController.cpp:342 > > > + const uint16_t* fileNameString = fromWebCoreString(fileName); > > > > What is fromWebCoreString ? > > fromWebCoreString() retrieves uint16 from WebCore String. Maybe should we rename it to uint16OfWebCoreString()? inline const uint16_t* fromWebCoreString(const String& str) { return reinterpret_cast<const uint16_t*>(str.characters()); } It might be clearer to just inline this function into all it's call sites. Having the body of the function makes it clear that the memory is still owned by the string... Up to you.
Created attachment 159650 [details] Patch
Created attachment 159686 [details] Patch
Comment on attachment 159686 [details] Patch Even better.
Comment on attachment 159686 [details] Patch Clearing flags on attachment: 159686 Committed r126222: <http://trac.webkit.org/changeset/126222>
All reviewed patches have been landed. Closing bug.