Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2521513006:
DevTools: allow array previews to show static array getters (Closed)
|
Created:
4 years, 1 month ago by luoe Modified:
4 years ago CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, pfeldman, kozyatinskiy+blink_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDevTools: allow array previews to show static array getters
This CL introduces the ability for the latter to properly render getter
properties when they are provided. Objects will continue to exclude show getter
properties in their preview, as is today.
By this change, ConsoleViewMessage's printArrayResult will no longer depend on
property descriptors, bringing us closer to moving all preview logic into
RemoteObjectPreviewFormatter.
Corresponding V8 CL: https://codereview.chromium.org/2508423002/
BUG=666882
Committed: https://crrev.com/644d46fd466d1d893ea6cc6b042a30bce66f3c98
Cr-Commit-Position: refs/heads/master@{#436797}
Patch Set 1 #
Total comments: 3
Patch Set 2 : rebase #Patch Set 3 : ac #Patch Set 4 : Punt needsManualRebaseline for later #Patch Set 5 : Rebaseline #
Messages
Total messages: 32 (23 generated)
Description was changed from ========== DevTools: allow array previews to show static array getters BUG=666882 ========== to ========== DevTools: allow array previews to show static array getters In order to bring the array preview formatter in ConsoleViewMessage closer to the logic in RemoteObjectPreviewFormatter, this CL introduces the ability for the latter to properly render getter properties when they are provided. BUG=666882 ==========
Description was changed from ========== DevTools: allow array previews to show static array getters In order to bring the array preview formatter in ConsoleViewMessage closer to the logic in RemoteObjectPreviewFormatter, this CL introduces the ability for the latter to properly render getter properties when they are provided. BUG=666882 ========== to ========== DevTools: allow array previews to show static array getters In order to bring the array preview formatter in ConsoleViewMessage closer to the logic in RemoteObjectPreviewFormatter, this CL introduces the ability for the latter to properly render getter properties when they are provided. Objects will continue to exclude show getter properties in their preview, as is today. BUG=666882 ==========
Description was changed from ========== DevTools: allow array previews to show static array getters In order to bring the array preview formatter in ConsoleViewMessage closer to the logic in RemoteObjectPreviewFormatter, this CL introduces the ability for the latter to properly render getter properties when they are provided. Objects will continue to exclude show getter properties in their preview, as is today. BUG=666882 ========== to ========== DevTools: allow array previews to show static array getters In order to bring the array preview formatter in ConsoleViewMessage closer to the logic in RemoteObjectPreviewFormatter, this CL introduces the ability for the latter to properly render getter properties when they are provided. Objects will continue to exclude show getter properties in their preview, as is today. BUG=666882 ==========
Description was changed from ========== DevTools: allow array previews to show static array getters In order to bring the array preview formatter in ConsoleViewMessage closer to the logic in RemoteObjectPreviewFormatter, this CL introduces the ability for the latter to properly render getter properties when they are provided. Objects will continue to exclude show getter properties in their preview, as is today. BUG=666882 ========== to ========== DevTools: allow array previews to show static array getters This CL introduces the ability for the latter to properly render getter properties when they are provided. Objects will continue to exclude show getter properties in their preview, as is today. By this change, ConsoleViewMessage's printArrayResult will no longer depend on property descriptors, bringing us closer to moving all preview logic into RemoteObjectPreviewFormatter. BUG=666882 ==========
luoe@chromium.org changed reviewers: + dgozman@chromium.org, kozyatinskiy@chromium.org
Please take a look. There is a corresponding CL that will include getter properties in the protocol's ObjectPreview here: https://codereview.chromium.org/2508423002/
Description was changed from ========== DevTools: allow array previews to show static array getters This CL introduces the ability for the latter to properly render getter properties when they are provided. Objects will continue to exclude show getter properties in their preview, as is today. By this change, ConsoleViewMessage's printArrayResult will no longer depend on property descriptors, bringing us closer to moving all preview logic into RemoteObjectPreviewFormatter. BUG=666882 ========== to ========== DevTools: allow array previews to show static array getters This CL introduces the ability for the latter to properly render getter properties when they are provided. Objects will continue to exclude show getter properties in their preview, as is today. By this change, ConsoleViewMessage's printArrayResult will no longer depend on property descriptors, bringing us closer to moving all preview logic into RemoteObjectPreviewFormatter. Corresponding V8 CL: https://codereview.chromium.org/2508423002/ BUG=666882 ==========
https://codereview.chromium.org/2521513006/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js (right): https://codereview.chromium.org/2521513006/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js:151: span.textContent = '(...)'; Let's make this clickable when we can.
After talking with chowse@ and kozyatinskiy@, we can leave this as '(...)', and strive to make previews more static. If we can easily re-add this functionality later, there is a new issue crbug.com/670520.
lgtm https://codereview.chromium.org/2521513006/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js (right): https://codereview.chromium.org/2521513006/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js:151: span.textContent = '(...)'; On 2016/11/24 00:38:49, dgozman wrote: > Let's make this clickable when we can. Let's just have a tooltip with explanation.
Thank you for the review https://codereview.chromium.org/2521513006/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js (right): https://codereview.chromium.org/2521513006/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js:151: span.textContent = '(...)'; Added as "The property is computed with a getter"
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...
lgtm
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 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: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
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, kozyatinskiy@chromium.org Link to the patchset: https://codereview.chromium.org/2521513006/#ps80001 (title: "Rebaseline")
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": 80001, "attempt_start_ts": 1481070938728930,
"parent_rev": "b8955342cba62749def1466332c55b2d87afed2e", "commit_rev":
"e679cccca4a7cb87ba403de0bcf7908ff40e5a28"}
Message was sent while issue was closed.
Description was changed from ========== DevTools: allow array previews to show static array getters This CL introduces the ability for the latter to properly render getter properties when they are provided. Objects will continue to exclude show getter properties in their preview, as is today. By this change, ConsoleViewMessage's printArrayResult will no longer depend on property descriptors, bringing us closer to moving all preview logic into RemoteObjectPreviewFormatter. Corresponding V8 CL: https://codereview.chromium.org/2508423002/ BUG=666882 ========== to ========== DevTools: allow array previews to show static array getters This CL introduces the ability for the latter to properly render getter properties when they are provided. Objects will continue to exclude show getter properties in their preview, as is today. By this change, ConsoleViewMessage's printArrayResult will no longer depend on property descriptors, bringing us closer to moving all preview logic into RemoteObjectPreviewFormatter. Corresponding V8 CL: https://codereview.chromium.org/2508423002/ BUG=666882 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== DevTools: allow array previews to show static array getters This CL introduces the ability for the latter to properly render getter properties when they are provided. Objects will continue to exclude show getter properties in their preview, as is today. By this change, ConsoleViewMessage's printArrayResult will no longer depend on property descriptors, bringing us closer to moving all preview logic into RemoteObjectPreviewFormatter. Corresponding V8 CL: https://codereview.chromium.org/2508423002/ BUG=666882 ========== to ========== DevTools: allow array previews to show static array getters This CL introduces the ability for the latter to properly render getter properties when they are provided. Objects will continue to exclude show getter properties in their preview, as is today. By this change, ConsoleViewMessage's printArrayResult will no longer depend on property descriptors, bringing us closer to moving all preview logic into RemoteObjectPreviewFormatter. Corresponding V8 CL: https://codereview.chromium.org/2508423002/ BUG=666882 Committed: https://crrev.com/644d46fd466d1d893ea6cc6b042a30bce66f3c98 Cr-Commit-Position: refs/heads/master@{#436797} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/644d46fd466d1d893ea6cc6b042a30bce66f3c98 Cr-Commit-Position: refs/heads/master@{#436797} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
