Currently InjectedScript operates ScriptValue/Object and is running within inspected page's context. It is safe to pass objects between injected script and the page. Frontend is running in its own context, entirely isolated from the inspected page. We should make sure that JS objects do not cross the boundary and I suggest that we enforce it via introducing explicit FrontendValue/Object classes (derived from ScriptValue/Object). InspectorFrontend then will only accept objects of this type.
Created attachment 56066 [details] [WIP Patch] Handing over to Ilya.
Created attachment 56874 [details] [patch] initial version. It is the first patch with implementation for InspectorValues classes. The changes for TimelineAgent, DOMAgent etc. will be processed separately.
Comment on attachment 56874 [details] [patch] initial version. WebCore/inspector/InspectorFrontend.h:53 + virtual void dispatchMessageToInspector(const String& message) = 0; We already have InspectorFrontendClient which serves as the frontend delegate, this method should go there. WebCore/inspector/InspectorValues.cpp:43 + JSONBuffer(size_t preallocate) : m_position(0), m_buffer(preallocate) { } Should be possible to use Vector instead of this class, it has capacity in addition to size. The only issue there may be fixed resize strategy. WebCore/inspector/InspectorValues.h:117 + explicit InspectorValue(bool value) : InspectorBaseValue(TypeBoolean) { m_boolValue = value; } should other constructors be explicit? WebCore/inspector/InspectorValues.h:103 + class InspectorValue : public InspectorBaseValue { I think this should be InspectorPromitiveValue or something, InspectorValue is too generic. Can we have tests for the JSON stuff?
Created attachment 57459 [details] [patch] Classes for native serialization to JSON.
Comment on attachment 57459 [details] [patch] Classes for native serialization to JSON. WebCore/inspector/InspectorValues.h:47 + class InspectorBaseValue : public RefCounted<InspectorBaseValue> { InspectorValue WebCore/inspector/InspectorValues.h:78 + class InspectorValue : public InspectorBaseValue { InspectorBasicValue?
Attachment 57459 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/2797010
Attachment 57459 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/2822008
Created attachment 57461 [details] [patch] second iteration InspectorValue -> InspectorBasicValue InspectorBaseValue -> InspectorValue
Attachment 57461 [details] did not build on mac: Build output: http://webkit-commit-queue.appspot.com/results/2803008
Attachment 57461 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/2750012
Created attachment 57463 [details] [patch] third iteration. With fixes for linux build.
Attachment 57463 [details] did not build on mac: Build output: http://webkit-commit-queue.appspot.com/results/2802012
Attachment 57461 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/2750013
Created attachment 57466 [details] [patch] fourth iteration. With fixed mac export list.
Comment on attachment 57466 [details] [patch] fourth iteration. With fixed mac export list. Clearing flags on attachment: 57466 Committed r60452: <http://trac.webkit.org/changeset/60452>
All reviewed patches have been landed. Closing bug.
Comment on attachment 57466 [details] [patch] fourth iteration. With fixed mac export list. > +++ b/WebCore/inspector/InspectorValues.cpp > + result.reserveCapacity(512); For my education, was there any particular surrounding the decision for 512? > +++ b/WebCore/inspector/InspectorValues.h > + unsigned length() { return m_data.size(); } I think this can this be const: unsigned length() const { ... } Otherwise really well done.This looks very clean!