WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2015-02-13 19:30:32 PST
<
rdar://problem/19836765
>
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
Yes.
http://trac.webkit.org/changeset/180594
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug