Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2593673002:
[Fetch API] Add use counter for Headers.prototype.getAll() (Closed)
DescriptionTo measue usage of Headers.prototype.getAll().
We need to add use counter for Headers.prototype.getAll().
BUG=645492
Committed: https://crrev.com/673306fb8007e128a1217dff1bfb22acc61262be
Cr-Commit-Position: refs/heads/master@{#440351}
Patch Set 1 #Patch Set 2 : merge conflict #Patch Set 3 : fix #
Messages
Total messages: 36 (21 generated)
Description was changed from ========== [Fech API] Add use counter for Headers.prototype.getAll() BUG=645492 ========== to ========== To measue usage of Headers.prototype.getAll(). We need to add use counter for Headers.prototype.getAll(). BUG=645492 ==========
corona10@gmail.com changed reviewers: + yhirano@chromium.org
yhirano@ PTAL. Is it right way to add use counter? I'm first to add it.
corona10@gmail.com changed reviewers: + jsbell@chromium.org
The CQ bit was checked by yhirano@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: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
On 2016/12/20 15:26:36, dhna wrote: > yhirano@ > PTAL. Is it right way to add use counter? > I'm first to add it. How about using Measure instead of MeasureAs=?
On 2016/12/22 03:52:25, yhirano wrote: > On 2016/12/20 15:26:36, dhna wrote: > > yhirano@ > > PTAL. Is it right way to add use counter? > > I'm first to add it. > > How about using Measure instead of MeasureAs=? yhirano@ Thanks I will change it. And I should update to fix conflict.
On 2016/12/22 03:53:27, dhna wrote: > On 2016/12/22 03:52:25, yhirano wrote: > > On 2016/12/20 15:26:36, dhna wrote: > > > yhirano@ > > > PTAL. Is it right way to add use counter? > > > I'm first to add it. > > > > How about using Measure instead of MeasureAs=? > > yhirano@ > Thanks I will change it. > And I should update to fix conflict. yhirano@ Done. PTAL
corona10@gmail.com changed reviewers: + jwd@chromium.org
On 2016/12/22 04:09:58, dhna wrote: > On 2016/12/22 03:53:27, dhna wrote: > > On 2016/12/22 03:52:25, yhirano wrote: > > > On 2016/12/20 15:26:36, dhna wrote: > > > > yhirano@ > > > > PTAL. Is it right way to add use counter? > > > > I'm first to add it. > > > > > > How about using Measure instead of MeasureAs=? > > > > yhirano@ > > Thanks I will change it. > > And I should update to fix conflict. > > yhirano@ > Done. > PTAL I meant using Measure without argument, like https://codereview.chromium.org/2581813002.
On 2016/12/22 04:11:37, yhirano wrote: > On 2016/12/22 04:09:58, dhna wrote: > > On 2016/12/22 03:53:27, dhna wrote: > > > On 2016/12/22 03:52:25, yhirano wrote: > > > > On 2016/12/20 15:26:36, dhna wrote: > > > > > yhirano@ > > > > > PTAL. Is it right way to add use counter? > > > > > I'm first to add it. > > > > > > > > How about using Measure instead of MeasureAs=? > > > > > > yhirano@ > > > Thanks I will change it. > > > And I should update to fix conflict. > > > > yhirano@ > > Done. > > PTAL > > I meant using Measure without argument, like > https://codereview.chromium.org/2581813002. [Measure] is documented at https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/So....
The CQ bit was checked by yhirano@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: chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
On 2016/12/22 04:12:22, yhirano wrote: > On 2016/12/22 04:11:37, yhirano wrote: > > On 2016/12/22 04:09:58, dhna wrote: > > > On 2016/12/22 03:53:27, dhna wrote: > > > > On 2016/12/22 03:52:25, yhirano wrote: > > > > > On 2016/12/20 15:26:36, dhna wrote: > > > > > > yhirano@ > > > > > > PTAL. Is it right way to add use counter? > > > > > > I'm first to add it. > > > > > > > > > > How about using Measure instead of MeasureAs=? > > > > > > > > yhirano@ > > > > Thanks I will change it. > > > > And I should update to fix conflict. > > > > > > yhirano@ > > > Done. > > > PTAL > > > > I meant using Measure without argument, like > > https://codereview.chromium.org/2581813002. > > [Measure] is documented at > https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/So.... yhirano@ Done. But 'V8Headers_GetAll_Method' is right name to add? Sorry, I'm not familiar with this work.
Thanks, lgtm.
The CQ bit was checked by corona10@gmail.com 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...
On 2016/12/22 04:37:50, yhirano wrote: > Thanks, lgtm. Thanks!! jwd@ PTAL for tools/metrics/histograms/histograms.xml
corona10@gmail.com changed reviewers: + haraken@chromium.org - jwd@chromium.org
On 2016/12/22 04:39:58, dhna wrote: > On 2016/12/22 04:37:50, yhirano wrote: > > Thanks, lgtm. > > Thanks!! > jwd@ > PTAL for tools/metrics/histograms/histograms.xml Sorry for mistake. haraken@ Can you take a look for tools/metrics/histograms/histograms.xml and third_party/WebKit/Source/core/frame/UseCounter.h?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM
The CQ bit was checked by corona10@gmail.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/12/22 07:26:10, haraken wrote: > LGTM haraken@ Thanks!
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1482391599702880,
"parent_rev": "2e83da58322c374eae4f7b66b5f0b3e0cca4b153", "commit_rev":
"9a1ba1ddc8ee72bcc47888a1285372c640da2595"}
Message was sent while issue was closed.
Description was changed from ========== To measue usage of Headers.prototype.getAll(). We need to add use counter for Headers.prototype.getAll(). BUG=645492 ========== to ========== To measue usage of Headers.prototype.getAll(). We need to add use counter for Headers.prototype.getAll(). BUG=645492 Review-Url: https://codereview.chromium.org/2593673002 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== To measue usage of Headers.prototype.getAll(). We need to add use counter for Headers.prototype.getAll(). BUG=645492 Review-Url: https://codereview.chromium.org/2593673002 ========== to ========== To measue usage of Headers.prototype.getAll(). We need to add use counter for Headers.prototype.getAll(). BUG=645492 Committed: https://crrev.com/673306fb8007e128a1217dff1bfb22acc61262be Cr-Commit-Position: refs/heads/master@{#440351} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/673306fb8007e128a1217dff1bfb22acc61262be Cr-Commit-Position: refs/heads/master@{#440351} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
