Bug 24524

Created attachment 28496 [details] Introduce ScriptObject and ScriptFunctionCall abstractions, v1. WebCore/ChangeLog | 29 ++++++ WebCore/GNUmakefile.am | 5 + WebCore/WebCore.pro | 2 + WebCore/WebCore.vcproj/WebCore.vcproj | 20 ++++ WebCore/WebCore.xcodeproj/project.pbxproj | 20 ++++ WebCore/WebCoreSources.bkl | 2 + WebCore/bindings/js/ScriptFunctionCall.cpp | 122 +++++++++++++++++++++++ WebCore/bindings/js/ScriptFunctionCall.h | 63 ++++++++++++ WebCore/bindings/js/ScriptObject.cpp | 63 ++++++++++++ WebCore/bindings/js/ScriptObject.h | 57 +++++++++++ WebCore/bindings/js/ScriptObjectQuarantine.h | 62 ++++++++++++ WebCore/inspector/InspectorController.cpp | 87 +--------------- WebCore/inspector/InspectorController.h | 4 - WebCore/inspector/InspectorDatabaseResource.cpp | 53 +++++++--- WebCore/inspector/InspectorDatabaseResource.h | 18 ++-- 15 files changed, 496 insertions(+), 111 deletions(-)

Why does ScriptObject contain an ExecState?

(In reply to comment #2) > Why does ScriptObject contain an ExecState? > I struggled with this and open to ideas. Current InspectorController holds on to both ContextRef and ObjectRef, and it seemed really appealing to keep them coupled in one object. But I understand that if we view ScriptObject as a more general abstraction, it seems less logical. I am very open to ideas here.

Comment on attachment 28496 [details] Introduce ScriptObject and ScriptFunctionCall abstractions, v1. Taking this out of review queue and marking obsolete. weinig schooled me and I am now on the right path.

Created attachment 28533 [details] Introduce ScriptObject and ScriptFunctionCall abstractions, v2. WebCore/ChangeLog | 29 ++++++ WebCore/GNUmakefile.am | 5 + WebCore/WebCore.pro | 2 + WebCore/WebCore.vcproj/WebCore.vcproj | 20 ++++ WebCore/WebCore.xcodeproj/project.pbxproj | 20 ++++ WebCore/WebCoreSources.bkl | 2 + WebCore/bindings/js/ScriptFunctionCall.cpp | 121 +++++++++++++++++++++++ WebCore/bindings/js/ScriptFunctionCall.h | 64 ++++++++++++ WebCore/bindings/js/ScriptObject.cpp | 43 ++++++++ WebCore/bindings/js/ScriptObject.h | 52 ++++++++++ WebCore/bindings/js/ScriptObjectQuarantine.h | 62 ++++++++++++ WebCore/inspector/InspectorController.cpp | 87 +---------------- WebCore/inspector/InspectorController.h | 4 - WebCore/inspector/InspectorDatabaseResource.cpp | 54 +++++++--- WebCore/inspector/InspectorDatabaseResource.h | 19 ++-- 15 files changed, 473 insertions(+), 111 deletions(-)

Sam, what do you think? Did I learn my ABCs? :)

Created attachment 28584 [details] Introduce ScriptObject and ScriptFunctionCall abstractions, v2.1. WebCore/ChangeLog | 29 ++++++ WebCore/GNUmakefile.am | 5 + WebCore/WebCore.pro | 2 + WebCore/WebCore.vcproj/WebCore.vcproj | 20 ++++ WebCore/WebCore.xcodeproj/project.pbxproj | 20 ++++ WebCore/WebCoreSources.bkl | 2 + WebCore/bindings/js/ScriptFunctionCall.cpp | 121 +++++++++++++++++++++++ WebCore/bindings/js/ScriptFunctionCall.h | 64 ++++++++++++ WebCore/bindings/js/ScriptObject.cpp | 43 ++++++++ WebCore/bindings/js/ScriptObject.h | 50 +++++++++ WebCore/bindings/js/ScriptObjectQuarantine.h | 62 ++++++++++++ WebCore/bindings/js/ScriptValue.h | 1 + WebCore/inspector/InspectorController.cpp | 87 +---------------- WebCore/inspector/InspectorController.h | 4 - WebCore/inspector/InspectorDatabaseResource.cpp | 54 +++++++--- WebCore/inspector/InspectorDatabaseResource.h | 19 ++-- 16 files changed, 472 insertions(+), 111 deletions(-)

New patch up, addressing some of the comments on IRC: * ScriptObject now inherits from ScriptValue. Pretty. * I thought about changing "bind/unbind" method names, but couldn't come up with anything that's not verbose. I am open to ideas here, but I think this is pretty intuitive already. * Going away from JSC API. I understand the dogfoodiness aspect of it, and I appreciate the desire. In fact, my using bindings functions was merely an attempt to homogenize the code base, not a direct desire to get rid of it. So, here's my idea: perhaps we could convert all Script* objects to use JSC API? This could be a nice little GSoC project? :)

Comment on attachment 28584 [details] Introduce ScriptObject and ScriptFunctionCall abstractions, v2.1. Looks good, only two comments. Don't indent the functions in the .cpp files inside the namespace block. And getQuarantinedScriptObject seems a little large to be an inline function, so that should likely have a .cpp file to contain the implementation.

Comments addressed, landed as http://trac.webkit.org/changeset/41684.