|
|
Description[inspector] move coverage related methods to profiler
R=dgozman@chromium.org, pfeldman@chromium.org
BUG=v8:5808
Review-Url: https://codereview.chromium.org/2715833003
Cr-Commit-Position: refs/heads/master@{#43426}
Committed: https://chromium.googlesource.com/v8/v8/+/64a563c97f84c474a3f8b9a6b02c68ec1728cfee
Patch Set 1 #Patch Set 2 : precise coverage works only when runtime is enabled #Patch Set 3 : added missing test #
Total comments: 2
Patch Set 4 : "true" -> true #Patch Set 5 : rebased #
Messages
Total messages: 59 (35 generated)
Description was changed from ========== [inspector] move coverage related methods to profiler These methods are available regardless is Profiler enabled or not. R=dgozman@chromium.org, pfeldman@chromium.org BUG=v8:5808 ========== to ========== [inspector] move coverage related methods to profiler These methods are available regardless is Profiler enabled or not. R=dgozman@chromium.org, pfeldman@chromium.org BUG=v8:5808 ==========
After offline discussion we decided that Profiler domain is better place for coverage related methods. Dmitry and Pavel, please take a look.
Description was changed from ========== [inspector] move coverage related methods to profiler These methods are available regardless is Profiler enabled or not. R=dgozman@chromium.org, pfeldman@chromium.org BUG=v8:5808 ========== to ========== [inspector] move coverage related methods to profiler R=dgozman@chromium.org, pfeldman@chromium.org BUG=v8:5808 ==========
kozyatinskiy@chromium.org changed reviewers: + alph@chromium.org
+Alexey for v8-profiler-agent refactoring review.
looks good, deferring to alph
lgtm https://codereview.chromium.org/2715833003/diff/40001/src/inspector/v8-profil... File src/inspector/v8-profiler-agent-impl.cc (right): https://codereview.chromium.org/2715833003/diff/40001/src/inspector/v8-profil... src/inspector/v8-profiler-agent-impl.cc:367: m_profiler->SetIdle(m_idle); Do you really need that? I bet m_idle is always false here.
https://codereview.chromium.org/2715833003/diff/40001/src/inspector/v8-profil... File src/inspector/v8-profiler-agent-impl.cc (right): https://codereview.chromium.org/2715833003/diff/40001/src/inspector/v8-profil... src/inspector/v8-profiler-agent-impl.cc:367: m_profiler->SetIdle(m_idle); On 2017/02/24 03:12:56, alph wrote: > Do you really need that? I bet m_idle is always false here. I'm not sure. I'll try to get rid of this flag in follow up CL to not mix refactoring with semantic changes.
The CQ bit was checked by kozyatinskiy@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.
Pavel or Dmitry, please take a look, I need owner lgtm for js_protocol.json.
yangguo@chromium.org changed reviewers: + yangguo@chromium.org
lgtm. thanks!
The CQ bit was checked by kozyatinskiy@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.
lgtm
The CQ bit was checked by kozyatinskiy@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from alph@chromium.org Link to the patchset: https://codereview.chromium.org/2715833003/#ps50010 (title: ""true" -> true")
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: v8_presubmit on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/35471)
The CQ bit was checked by kozyatinskiy@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org, alph@chromium.org, yangguo@chromium.org Link to the patchset: https://codereview.chromium.org/2715833003/#ps70001 (title: "rebased")
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: v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/21613) v8_linux_dbg_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/b...)
The CQ bit was checked by kozyatinskiy@chromium.org
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: v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...) v8_linux_nodcheck_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng_tr...)
The CQ bit was checked by kozyatinskiy@chromium.org
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: v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...) v8_linux64_asan_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng_trig...)
The CQ bit was checked by kozyatinskiy@chromium.org
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: v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...) v8_linux64_asan_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng_trig...)
The CQ bit was checked by kozyatinskiy@chromium.org
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: v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...) v8_linux64_asan_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng_trig...) v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/21621) v8_linux_dbg_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/b...) v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...) v8_linux_nodcheck_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng_tr...)
The CQ bit was checked by kozyatinskiy@chromium.org
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: v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...) v8_linux_nodcheck_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng_tr...) v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/18060) v8_mac_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng_triggered/bui...)
The CQ bit was checked by kozyatinskiy@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 kozyatinskiy@chromium.org
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": 70001, "attempt_start_ts": 1487990585473510, "parent_rev": "6b7650f03944238afd67274cec634cc511524562", "commit_rev": "64a563c97f84c474a3f8b9a6b02c68ec1728cfee"}
Message was sent while issue was closed.
Description was changed from ========== [inspector] move coverage related methods to profiler R=dgozman@chromium.org, pfeldman@chromium.org BUG=v8:5808 ========== to ========== [inspector] move coverage related methods to profiler R=dgozman@chromium.org, pfeldman@chromium.org BUG=v8:5808 Review-Url: https://codereview.chromium.org/2715833003 Cr-Commit-Position: refs/heads/master@{#43426} Committed: https://chromium.googlesource.com/v8/v8/+/64a563c97f84c474a3f8b9a6b02c68ec172... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:70001) as https://chromium.googlesource.com/v8/v8/+/64a563c97f84c474a3f8b9a6b02c68ec172... |