Bug 25063

Created attachment 29291 [details] Refactor InspectorController to use ScriptObject/State, v1 WebCore/ChangeLog | 24 ++ WebCore/GNUmakefile.am | 1 + WebCore/WebCore.pro | 1 + WebCore/WebCore.vcproj/WebCore.vcproj | 4 + WebCore/WebCore.xcodeproj/project.pbxproj | 4 + WebCore/WebCoreSources.bkl | 1 + .../bindings/js/JSInspectorControllerCustom.cpp | 2 +- WebCore/bindings/js/ScriptObject.cpp | 30 +++ WebCore/bindings/js/ScriptObject.h | 10 + WebCore/bindings/js/ScriptState.cpp | 44 ++++ WebCore/bindings/js/ScriptState.h | 3 + WebCore/inspector/InspectorController.cpp | 232 ++++++-------------- WebCore/inspector/InspectorController.h | 16 +- 13 files changed, 197 insertions(+), 175 deletions(-)

Comment on attachment 29291 [details] Refactor InspectorController to use ScriptObject/State, v1 It would be nice if !m_webInspector meant m_webInspector.hasNoValue(). It would make lines like the following more readable. if (m_scriptState && !m_webInspector.hasNoValue()) What are we losing with the removal of handleException? Are exceptions that happen in the Inspector JavaScript code still reported in it's Console?

Comment on attachment 29291 [details] Refactor InspectorController to use ScriptObject/State, v1 Good idea on hasNoValue -> !value. As for handleException -- you have reminded me I need to be careful and get this straight. The idea was that any set/get/call failures in Script* world use reportException, but this obviously has some re-entrancy implications for ConsoleMessage::addToConsole. Updated patch coming up.

Created attachment 29314 [details] Refactor InspectorController to use ScriptObject/State, v2 WebCore/ChangeLog | 35 +++ WebCore/GNUmakefile.am | 1 + WebCore/WebCore.pro | 1 + WebCore/WebCore.vcproj/WebCore.vcproj | 4 + WebCore/WebCore.xcodeproj/project.pbxproj | 4 + WebCore/WebCoreSources.bkl | 1 + .../bindings/js/JSInspectorControllerCustom.cpp | 2 +- WebCore/bindings/js/ScriptFunctionCall.cpp | 20 ++- WebCore/bindings/js/ScriptFunctionCall.h | 5 +- WebCore/bindings/js/ScriptObject.cpp | 53 ++++- WebCore/bindings/js/ScriptObject.h | 10 + WebCore/bindings/js/ScriptState.cpp | 44 ++++ WebCore/bindings/js/ScriptState.h | 3 + WebCore/inspector/ConsoleMessage.cpp | 4 +- WebCore/inspector/InspectorController.cpp | 235 ++++++-------------- WebCore/inspector/InspectorController.h | 17 +- 16 files changed, 244 insertions(+), 195 deletions(-)

Comment on attachment 29314 [details] Refactor InspectorController to use ScriptObject/State, v2 Oops. Never mind -- need to tweak this one.

Created attachment 29315 [details] Refactor InspectorController to use ScriptObject/State, v2.1 WebCore/ChangeLog | 35 +++ WebCore/GNUmakefile.am | 1 + WebCore/WebCore.pro | 1 + WebCore/WebCore.vcproj/WebCore.vcproj | 4 + WebCore/WebCore.xcodeproj/project.pbxproj | 4 + WebCore/WebCoreSources.bkl | 1 + .../bindings/js/JSInspectorControllerCustom.cpp | 2 +- WebCore/bindings/js/ScriptFunctionCall.cpp | 20 ++- WebCore/bindings/js/ScriptFunctionCall.h | 5 +- WebCore/bindings/js/ScriptObject.cpp | 53 ++++- WebCore/bindings/js/ScriptObject.h | 10 + WebCore/bindings/js/ScriptState.cpp | 44 ++++ WebCore/bindings/js/ScriptState.h | 3 + WebCore/inspector/ConsoleMessage.cpp | 4 +- WebCore/inspector/InspectorController.cpp | 231 ++++++-------------- WebCore/inspector/InspectorController.h | 17 +- 16 files changed, 242 insertions(+), 193 deletions(-)

Updated and improved: * I opted to add hasWebInspector() to simplify condition logic. I promise it wasn't because I was lazy to add the operators to ScriptValue :). I just thought it would be a bit more descriptive as to what is actually going on. * I made sure that exceptions are reported and any re-entrancy issues are addressed.

One thing to mention -- I consciously changed handling of exceptions a little bit. They are now routed to inspector of inspector. In other words, exceptions in the inspector JS will be seen in the console of its inspector -- to see them, you need to inspect the inspector! :)

Landed as http://trac.webkit.org/changeset/42295 and http://trac.webkit.org/changeset/42296.