|
|
Chromium Code Reviews|
Created:
4 years, 2 months ago by panicker Modified:
4 years, 2 months ago CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, asvitkine+watch_chromium.org, devtools-reviews_chromium.org, blink-reviews, kozyatinskiy+blink_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd use counter for Long Task Observer
BUG=635596
Committed: https://crrev.com/dc614078c8fb2f9144150d899c6b1a782b24cb27
Cr-Commit-Position: refs/heads/master@{#423082}
Patch Set 1 #
Total comments: 2
Patch Set 2 : address review comment #Patch Set 3 : sync to head and update usecounter value #
Total comments: 2
Patch Set 4 : sync and rebase update #
Messages
Total messages: 29 (14 generated)
panicker@chromium.org changed reviewers: + caseq@chromium.org
panicker@chromium.org changed reviewers: + esprehn@chromium.org
https://codereview.chromium.org/2387963002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp (right): https://codereview.chromium.org/2387963002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp:51: UseCounter::count(m_inspectedFrames->root()->domWindow()->document(), This requires a non-null document. You can, however, pass Frame* to count, which would make thins a bit simpler here.
PTAL https://codereview.chromium.org/2387963002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp (right): https://codereview.chromium.org/2387963002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp:51: UseCounter::count(m_inspectedFrames->root()->domWindow()->document(), On 2016/10/03 17:05:34, caseq wrote: > This requires a non-null document. You can, however, pass Frame* to count, which > would make thins a bit simpler here. Done.
lgtm
panicker@chromium.org changed reviewers: + isherman@chromium.org
+isherman for histograms.
histograms.xml lgtm
panicker@chromium.org changed reviewers: + pfeldman@chromium.org
Pavel - mind approving for OWNERS?
The CQ bit was checked by panicker@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 https://codereview.chromium.org/2387963002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp (right): https://codereview.chromium.org/2387963002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp:46: UseCounter::count(m_inspectedFrames->root(), UseCounter::LongTaskObserver); You probably need to rebaseline this.
https://codereview.chromium.org/2387963002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp (right): https://codereview.chromium.org/2387963002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp:46: UseCounter::count(m_inspectedFrames->root(), UseCounter::LongTaskObserver); On 2016/10/04 20:26:28, pfeldman wrote: > You probably need to rebaseline this. Yep, done.
Description was changed from ========== Add use counter for Long Task Observer BUG=635596 ========== to ========== Add use counter for Long Task Observer BUG=635596 ==========
panicker@chromium.org changed reviewers: - esprehn@chromium.org
The CQ bit was checked by panicker@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org, isherman@chromium.org, pfeldman@chromium.org Link to the patchset: https://codereview.chromium.org/2387963002/#ps60001 (title: "sync and rebase update")
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
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_...)
The CQ bit was checked by panicker@chromium.org
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.
Description was changed from ========== Add use counter for Long Task Observer BUG=635596 ========== to ========== Add use counter for Long Task Observer BUG=635596 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Add use counter for Long Task Observer BUG=635596 ========== to ========== Add use counter for Long Task Observer BUG=635596 Committed: https://crrev.com/dc614078c8fb2f9144150d899c6b1a782b24cb27 Cr-Commit-Position: refs/heads/master@{#423082} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/dc614078c8fb2f9144150d899c6b1a782b24cb27 Cr-Commit-Position: refs/heads/master@{#423082} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
