RESOLVED FIXED 141587
Web Inspector: Make Getter/Setter RemoteObject property and ObjectPreview handling consistent
https://bugs.webkit.org/show_bug.cgi?id=141587
Summary Web Inspector: Make Getter/Setter RemoteObject property and ObjectPreview han...
Joseph Pecoraro
Reported 2015-02-13 19:29:58 PST
* SUMMARY Make Getter/Setter RemoteObject property handling consistent. I'm going to use the term "accessor" for a property descriptor that is a getter/setter not a value. Currently InjectedScriptSource does its own conversions of accessors into value descriptors. This gains us values during previews, but we lose the accessor function values later. We should really standardize on Getter/Setter handling in RemoteObject properties and Object previews. * THOUGHTS - Getters can have side-effects. We should not always evaluate them. - However, Native getters (DOM Bindings) should not have side-effects. We can get their values. - Frontend shows these properties in a few ways: - (1) Object previews - quickly show some properties/values of the object - (2) Object expansion - immediately show all properties/values of the object - (3) API expansion - show the existence of getters/setters in the prototype chain * PROPOSAL Anytime an accessor has a Native Getter, we should evaluate the native getter to get the value. => treat as a Value basically everywhere => treat as a Getter during API expansion, since it is a getter (1) Object Previews: (Runtime.evaluate preview=true) - Value Descriptor => Property with "value/type/subtype". - Accessor with Native Getter => PropertyPreview with "value/type/subtype". No different than a value. - Accessor with Non-Native Getter => PropertyPreview type "accessor". Frontend should be able to turn this into a value via RemoteObject. - Accessor with Setter => setters are ignored for previews, but they will mark the preview as lossy. (2) Object Expansion: (Runtime.getProperties ownAndGetterProperties=true) - Value Descriptor => PropertyDescriptor with value - Accessor with Native Getter => PropertyDescriptor with "nativeGetterValue" and "get". Treat the same as a value. - Accessor with Non-Native Getter => PropertyDescriptor with "get". Frontend should be able to turn this into a value via RemoteObject. - Accessor with Setter => PropertyDescriptor with "set". Frontend could show a UI for setting this value if desired. (3) API Expansion: (Runtime.getProperties ownProperties=true, but this is in the API part of an object, typically a prototype) - Value Descriptor => PropertyDescriptor with value. Show "prop: value". - Accessor with Native Getter => PropertyDescriptor with "nativeGetterValue" and "get". Since this is a getter, show "get foo()" - Accessor with Non-Native Getter => PropertyDescriptor with "get". Since this is a getter, show "get foo()" - Accessor with Setter => PropertyDescriptor with "set". Since this is a setter, show "set foo(x)" --- We might want to reconsider ownAndGetterProperties to just be "ownAndAccessors", or perhaps a new API alongside getProperties, like "getVisibleProperties" for the object expansion portion. We haven't shipped this part of the protocol yet.
Attachments
[PATCH] Proposed Fix (43.33 KB, patch)
2015-02-24 13:44 PST, Joseph Pecoraro
timothy: review+
Radar WebKit Bug Importer
Comment 1 2015-02-13 19:30:32 PST
Joseph Pecoraro
Comment 2 2015-02-24 13:44:25 PST
Created attachment 247263 [details] [PATCH] Proposed Fix
Timothy Hatcher
Comment 3 2015-02-24 13:58:36 PST
Comment on attachment 247263 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=247263&action=review > Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:189 > + ScriptDebugServer::PauseOnExceptionsState previousPauseOnExceptionsState = setPauseOnExceptionsState(m_scriptDebugServer, ScriptDebugServer::DontPauseOnExceptions); A helper for this would be useful, something like muteConsole or combined with that. > Source/WebInspectorUI/UserInterface/Models/PropertyDescriptor.js:122 > + get isNativeGetterValue() We usually drop the is prefix for getters. Is Value suffix needed?
Joseph Pecoraro
Comment 4 2015-02-24 15:16:26 PST
> > Source/WebInspectorUI/UserInterface/Models/PropertyDescriptor.js:122 > > + get isNativeGetterValue() > > We usually drop the is prefix for getters. Is Value suffix needed? I'll drop both. I wanted to avoid possible confusion with types but it shouldn't be too bad.
Timothy Hatcher
Comment 5 2015-02-28 10:43:54 PST
Did this land with another change?
Timothy Hatcher
Comment 6 2015-02-28 15:57:24 PST
Note You need to log in before you can comment on or make changes to this bug.