I am working on getting rig of DOM agent pushes - everything should happen upon request. This patch changes the way we handle inspect: instead of pushing nodes, we are telling front-end that inspect element has been requested. It is then up to front-end to request dom nodes and focus them in the tree. I also made inspect() work in a generic manner, using same routines for nodes, databases, storages and potentially new elements.
Created attachment 83494 [details] Patch
Attachment 83494 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7982172
Created attachment 83496 [details] Patch
Created attachment 83546 [details] Patch
Comment on attachment 83546 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=83546&action=review > Source/WebCore/bindings/js/JSInjectedScriptHostCustom.cpp:174 > +#if ENABLE(DATABASE) Should be 2 lines above? > Source/WebCore/bindings/js/JSInjectedScriptHostCustom.cpp:186 > +#if ENABLE(DOM_STORAGE) same here > Source/WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp:218 > if (args.Length() < 1) Why is this not under the #if guard below? > Source/WebCore/inspector/InjectedScript.cpp:176 > +void InjectedScript::inspectNode(Node* node) There are no nodes in the worker context, we will need to find a way to get rid of this method along with Frame.h dependency there. > Source/WebCore/inspector/InjectedScript.cpp:182 > + function.appendArgument(InjectedScriptHost::nodeAsScriptValue(mainWorldScriptState(frame), node)); You should give the state from the m_injectedScriptObject, shouldn't you? > Source/WebCore/inspector/InjectedScriptSource.js:70 > + if (arguments.length === 0) What is this check for? If you need to check whether the object is valid you should check it explicitely. > Source/WebCore/inspector/InjectedScriptSource.js:213 > + releaseObject: function(objectId) Probably we could switch from the object groups to this method? > Source/WebCore/inspector/InspectorDatabaseResource.cpp:41 > +static long nextUnusedId = 1; What's the reason for this change?
Comment on attachment 83546 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=83546&action=review >> Source/WebCore/bindings/js/JSInjectedScriptHostCustom.cpp:174 >> +#if ENABLE(DATABASE) > > Should be 2 lines above? No, this is a bug fix since these methods are called from within Javascript unconditionally. I actually fixed callframes as well. >> Source/WebCore/bindings/js/JSInjectedScriptHostCustom.cpp:186 >> +#if ENABLE(DOM_STORAGE) > > same here ditto >> Source/WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp:218 >> if (args.Length() < 1) > > Why is this not under the #if guard below? ditto >> Source/WebCore/inspector/InjectedScript.cpp:182 >> + function.appendArgument(InjectedScriptHost::nodeAsScriptValue(mainWorldScriptState(frame), node)); > > You should give the state from the m_injectedScriptObject, shouldn't you? Done. >> Source/WebCore/inspector/InjectedScriptSource.js:70 >> + if (arguments.length === 0) > > What is this check for? If you need to check whether the object is valid you should check it explicitely. This is correct - I should be able to inspect and dump undefined, null, etc. This check did not change. >> Source/WebCore/inspector/InspectorDatabaseResource.cpp:41 >> +static long nextUnusedId = 1; > > What's the reason for this change? Consistency. All ids are 1-based longs.
Committed r79556: <http://trac.webkit.org/changeset/79556>