|
|
Chromium Code Reviews
DescriptionUpdated error message in PerformanceObserver.observe
BUG=662210
Committed: https://crrev.com/920e411ac7f876467faa8454256578d103e0304b
Cr-Commit-Position: refs/heads/master@{#431165}
Patch Set 1 #
Total comments: 2
Patch Set 2 : resolved comments #Messages
Total messages: 21 (13 generated)
The CQ bit was checked by sunjian@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...
sunjian@chromium.org changed reviewers: + panicker@google.com
panicker@chromium.org changed reviewers: + panicker@chromium.org, yoav@yoav.ws
LGTM. +Yoav for Owners
LGTM % suggestion for a slightly clearer error message https://codereview.chromium.org/2468063006/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/timing/PerformanceObserver.cpp (right): https://codereview.chromium.org/2468063006/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/timing/PerformanceObserver.cpp:52: "A Performance Observer MUST have a valid entryTypes attribute."); Just to clarify, if one of the entries is valid, this doesn't throw, right? If so, maybe the error thrown should reflect that? "A Performance Observer MUST have at least one valid entryType in its entryTypes attribute."?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
https://codereview.chromium.org/2468063006/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/timing/PerformanceObserver.cpp (right): https://codereview.chromium.org/2468063006/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/timing/PerformanceObserver.cpp:52: "A Performance Observer MUST have a valid entryTypes attribute."); On 2016/11/04 21:49:16, Yoav Weiss wrote: > Just to clarify, if one of the entries is valid, this doesn't throw, right? > If so, maybe the error thrown should reflect that? "A Performance Observer MUST > have at least one valid entryType in its entryTypes attribute."? Done.
The CQ bit was checked by sunjian@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 panicker@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yoav@yoav.ws, panicker@chromium.org Link to the patchset: https://codereview.chromium.org/2468063006/#ps20001 (title: "resolved comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Updated error message in PerformanceObserver.observe BUG=662210 ========== to ========== Updated error message in PerformanceObserver.observe BUG=662210 Committed: https://crrev.com/920e411ac7f876467faa8454256578d103e0304b Cr-Commit-Position: refs/heads/master@{#431165} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/920e411ac7f876467faa8454256578d103e0304b Cr-Commit-Position: refs/heads/master@{#431165} |
