Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2508423002:
Add getter properties to array entry previews (Closed)
DescriptionAdd getter properties to array entry previews
Getter properties are not currently included in the protocol's
Runtime.ObjectPreview. DevTools currently shows getter properties
when evaluating arrays in the console, and this CL brings them into
the preview generated for RemoteObjects.
Corresponding DevTools CL: https://codereview.chromium.org/2521513006/
BUG=666882
Committed: https://crrev.com/80bcbccc676700b7ef66bac1ee581621b6889b04
Cr-Commit-Position: refs/heads/master@{#41565}
Patch Set 1 #Patch Set 2 : add test #Patch Set 3 : Remove inherited props, have getters for all #Patch Set 4 : just getters #
Total comments: 4
Patch Set 5 : ac #Patch Set 6 : ac #Patch Set 7 : rebase #
Dependent Patchsets: Messages
Total messages: 26 (17 generated)
Description was changed from ========== Add inherited and getter properties to array entry previews BUG= ========== to ========== Add inherited and getter properties to array entry previews In the past, inherited and getter array entries were introduced in previews via a workaround. When evaluating a remote array, the generated preview was discarded by ConsoleViewMessage's formatParameterAsArray(). Instead, the function made a second call to the backend with getAllProperties() to get these missing properties. While this fixed the functionality for evaluated arrays, it did not affect arrays printed with the object formatter (e.g. console.log(arr)). This CL fixes the preview generator logic in injected-script-source to include these missing properties so that both array formatters will get them. BUG=320632, 666882 ==========
luoe@chromium.org changed reviewers: + dgozman@chromium.org, kozyatinskiy@chromium.org
This should land after this CL, in which DevTools shows getter array entries. https://codereview.chromium.org/2514893004/
Please take a look
Description was changed from ========== Add inherited and getter properties to array entry previews In the past, inherited and getter array entries were introduced in previews via a workaround. When evaluating a remote array, the generated preview was discarded by ConsoleViewMessage's formatParameterAsArray(). Instead, the function made a second call to the backend with getAllProperties() to get these missing properties. While this fixed the functionality for evaluated arrays, it did not affect arrays printed with the object formatter (e.g. console.log(arr)). This CL fixes the preview generator logic in injected-script-source to include these missing properties so that both array formatters will get them. BUG=320632, 666882 ========== to ========== DO NOT COMMIT This wouldn't work when showing a preview of an entry of a Map/Set. Map/Set entries that have accessors don't have references to themselves, so we cannot generate the '(...)' accessor. This means turning on accessors in object previews would introduce inconsistency. Add inherited and getter properties to array entry previews In the past, inherited and getter array entries were introduced in previews via a workaround. When evaluating a remote array, the generated preview was discarded by ConsoleViewMessage's formatParameterAsArray(). Instead, the function made a second call to the backend with getAllProperties() to get these missing properties. While this fixed the functionality for evaluated arrays, it did not affect arrays printed with the object formatter (e.g. console.log(arr)). This CL fixes the preview generator logic in injected-script-source to include these missing properties so that both array formatters will get them. BUG=320632, 666882 ==========
Message was sent while issue was closed.
luoe@chromium.org changed reviewers: - dgozman@chromium.org, kozyatinskiy@chromium.org
Message was sent while issue was closed.
Description was changed from ========== DO NOT COMMIT This wouldn't work when showing a preview of an entry of a Map/Set. Map/Set entries that have accessors don't have references to themselves, so we cannot generate the '(...)' accessor. This means turning on accessors in object previews would introduce inconsistency. Add inherited and getter properties to array entry previews In the past, inherited and getter array entries were introduced in previews via a workaround. When evaluating a remote array, the generated preview was discarded by ConsoleViewMessage's formatParameterAsArray(). Instead, the function made a second call to the backend with getAllProperties() to get these missing properties. While this fixed the functionality for evaluated arrays, it did not affect arrays printed with the object formatter (e.g. console.log(arr)). This CL fixes the preview generator logic in injected-script-source to include these missing properties so that both array formatters will get them. BUG=320632, 666882 ========== to ========== Add getter properties to array entry previews Getter properties are not currently included in the protocol's Runtime.ObjectPreview. DevTools currently shows getter properties when evaluating arrays in the console, and this CL brings them into the preview generated for RemoteObjects. BUG=320632, 666882 ==========
Message was sent while issue was closed.
Description was changed from ========== Add getter properties to array entry previews Getter properties are not currently included in the protocol's Runtime.ObjectPreview. DevTools currently shows getter properties when evaluating arrays in the console, and this CL brings them into the preview generated for RemoteObjects. BUG=320632, 666882 ========== to ========== Add getter properties to array entry previews Getter properties are not currently included in the protocol's Runtime.ObjectPreview. DevTools currently shows getter properties when evaluating arrays in the console, and this CL brings them into the preview generated for RemoteObjects. BUG=666882 ==========
luoe@chromium.org changed reviewers: + dgozman@chromium.org, kozyatinskiy@chromium.org
I've reopened this issue, which has a corresponding front end change in DevTools: https://codereview.chromium.org/2521513006/ Once that ^ lands, the front end will be able to handle accessor types in previews. PTAL
Description was changed from ========== Add getter properties to array entry previews Getter properties are not currently included in the protocol's Runtime.ObjectPreview. DevTools currently shows getter properties when evaluating arrays in the console, and this CL brings them into the preview generated for RemoteObjects. BUG=666882 ========== to ========== Add getter properties to array entry previews Getter properties are not currently included in the protocol's Runtime.ObjectPreview. DevTools currently shows getter properties when evaluating arrays in the console, and this CL brings them into the preview generated for RemoteObjects. Corresponding DevTools CL: https://codereview.chromium.org/2521513006/ BUG=666882 ==========
lgtm https://codereview.chromium.org/2508423002/diff/60001/src/inspector/injected-... File src/inspector/injected-script-source.js (right): https://codereview.chromium.org/2508423002/diff/60001/src/inspector/injected-... src/inspector/injected-script-source.js:944: // Ignore computed properties unless they are getters. ... they have getters. https://codereview.chromium.org/2508423002/diff/60001/test/inspector/runtime/... File test/inspector/runtime/evaluate-with-generate-preview.js (right): https://codereview.chromium.org/2508423002/diff/60001/test/inspector/runtime/... test/inspector/runtime/evaluate-with-generate-preview.js:17: Object.defineProperty(arr, 1, { Let's add a property with only a setter, and with both getter and setter in addition to this one.
https://codereview.chromium.org/2508423002/diff/60001/src/inspector/injected-... File src/inspector/injected-script-source.js (right): https://codereview.chromium.org/2508423002/diff/60001/src/inspector/injected-... src/inspector/injected-script-source.js:944: // Ignore computed properties unless they are getters. On 2016/11/24 00:33:32, dgozman wrote: > ... they have getters. Done. https://codereview.chromium.org/2508423002/diff/60001/test/inspector/runtime/... File test/inspector/runtime/evaluate-with-generate-preview.js (right): https://codereview.chromium.org/2508423002/diff/60001/test/inspector/runtime/... test/inspector/runtime/evaluate-with-generate-preview.js:17: Object.defineProperty(arr, 1, { On 2016/11/24 00:33:32, dgozman wrote: > Let's add a property with only a setter, and with both getter and setter in > addition to this one. Done.
lgtm
The CQ bit was checked by luoe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by luoe@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org Link to the patchset: https://codereview.chromium.org/2508423002/#ps120001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 120001, "attempt_start_ts": 1481149758629010,
"parent_rev": "412c30773a57300114728d65c335b41805556d29", "commit_rev":
"fdb98f082315cb0f8bf6170d307c08be7109a85f"}
Message was sent while issue was closed.
Description was changed from ========== Add getter properties to array entry previews Getter properties are not currently included in the protocol's Runtime.ObjectPreview. DevTools currently shows getter properties when evaluating arrays in the console, and this CL brings them into the preview generated for RemoteObjects. Corresponding DevTools CL: https://codereview.chromium.org/2521513006/ BUG=666882 ========== to ========== Add getter properties to array entry previews Getter properties are not currently included in the protocol's Runtime.ObjectPreview. DevTools currently shows getter properties when evaluating arrays in the console, and this CL brings them into the preview generated for RemoteObjects. Corresponding DevTools CL: https://codereview.chromium.org/2521513006/ BUG=666882 ==========
Message was sent while issue was closed.
Committed patchset #7 (id:120001)
Message was sent while issue was closed.
Description was changed from ========== Add getter properties to array entry previews Getter properties are not currently included in the protocol's Runtime.ObjectPreview. DevTools currently shows getter properties when evaluating arrays in the console, and this CL brings them into the preview generated for RemoteObjects. Corresponding DevTools CL: https://codereview.chromium.org/2521513006/ BUG=666882 ========== to ========== Add getter properties to array entry previews Getter properties are not currently included in the protocol's Runtime.ObjectPreview. DevTools currently shows getter properties when evaluating arrays in the console, and this CL brings them into the preview generated for RemoteObjects. Corresponding DevTools CL: https://codereview.chromium.org/2521513006/ BUG=666882 Committed: https://crrev.com/80bcbccc676700b7ef66bac1ee581621b6889b04 Cr-Commit-Position: refs/heads/master@{#41565} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/80bcbccc676700b7ef66bac1ee581621b6889b04 Cr-Commit-Position: refs/heads/master@{#41565} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
