Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
4 years, 2 months ago by kozy Modified:
4 years, 1 month ago Reviewers:
dgozman, Yang CC:
v8-reviews_googlegroups.com, Yang, devtools-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[inspector] migrate stepping related methods to debug-interface
* introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method.
Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid.
BUG=chromium:652939, v8:5510
R=yangguo@chromium.org,dgozman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
Committed: https://crrev.com/24e5dfb546d01394f375b61d55862201fa7df872
Cr-Commit-Position: refs/heads/master@{#40483}
Patch Set 1 #Patch Set 2 : rebased #
Total comments: 2
Patch Set 3 : addressed comments #Patch Set 4 : we use ClearStepping to cancel stepFrame #
Created: 4 years, 2 months ago
Messages
Total messages: 24 (12 generated)
Description was changed from ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 ========== to ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 R=yangguo@chromium.org,dgozman@chromium.org ==========
kozyatinskiy@chromium.org changed reviewers: + dgozman@chromium.org, yangguo@chromium.org
Yang and Dmitry, please take a look.
inspector lgtm, but this needs rebase
lgtm https://codereview.chromium.org/2423153002/diff/20001/src/api.cc File src/api.cc (right): https://codereview.chromium.org/2423153002/diff/20001/src/api.cc#newcode8820 src/api.cc:8820: CHECK(isolate->debug()->CheckExecutionState(isolate->debug()->break_id())); Can we introduce a new version of CheckExecutionState that does not take a break id? The current one would just compare debug->break_id() with itself.
all done. https://codereview.chromium.org/2423153002/diff/20001/src/api.cc File src/api.cc (right): https://codereview.chromium.org/2423153002/diff/20001/src/api.cc#newcode8820 src/api.cc:8820: CHECK(isolate->debug()->CheckExecutionState(isolate->debug()->break_id())); On 2016/10/19 04:38:56, Yang wrote: > Can we introduce a new version of CheckExecutionState that does not take a break > id? The current one would just compare debug->break_id() with itself. Done.
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, yangguo@chromium.org Link to the patchset: https://codereview.chromium.org/2423153002/#ps40001 (title: "addressed 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.
Description was changed from ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 R=yangguo@chromium.org,dgozman@chromium.org ========== to ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 R=yangguo@chromium.org,dgozman@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://chromiumcodereview.appspot.com/2441583002/ by machenbach@chromium.org. The reason for reverting is: https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/bui... https://github.com/v8/v8/wiki/Blink-layout-tests.
Message was sent while issue was closed.
We use cancelStepping to cancel stepFrame action in following scenario: - user set NativeBreakpoint (e.g. on click). - user blackbox onclick handler. - user execute element.click(). Debugger pauses in blackboxed handler then debugger agent will try to step to not blackboxed code by stepFrame action. If we don't clear stepping after handler code end then we will pause in the line after click call. I'll try to avoid this not-on-pause clearStepping call in new blackboxing implementation which is coming. But now we can remove check in DebugInterface::ClearStepping method and left current behavior as is.
Message was sent while issue was closed.
Description was changed from ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 R=yangguo@chromium.org,dgozman@chromium.org ========== to ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 R=yangguo@chromium.org,dgozman@chromium.org ==========
Description was changed from ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 R=yangguo@chromium.org,dgozman@chromium.org ========== to ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 R=yangguo@chromium.org,dgozman@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ==========
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, yangguo@chromium.org Link to the patchset: https://codereview.chromium.org/2423153002/#ps60001 (title: "we use ClearStepping to cancel stepFrame")
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 ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 R=yangguo@chromium.org,dgozman@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ========== to ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 R=yangguo@chromium.org,dgozman@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 R=yangguo@chromium.org,dgozman@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ========== to ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 R=yangguo@chromium.org,dgozman@chromium.org Committed: https://crrev.com/859eddbdefdd99a25164f2286b33a39051356ca4 Cr-Commit-Position: refs/heads/master@{#40450} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/859eddbdefdd99a25164f2286b33a39051356ca4 Cr-Commit-Position: refs/heads/master@{#40450}
Message was sent while issue was closed.
Description was changed from ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 R=yangguo@chromium.org,dgozman@chromium.org Committed: https://crrev.com/859eddbdefdd99a25164f2286b33a39051356ca4 Cr-Commit-Position: refs/heads/master@{#40450} ========== to ========== [inspector] migrate stepping related methods to debug-interface * introduced DebugInterface::PrepareStep and DebugInterface::ClearStepping method. Inspector calls these methods only on pause and not interseted in calling this for not current break_id so we don't need to expose debug interface with break_id argument and can only check that current break_id is valid. BUG=chromium:652939,v8:5510 R=yangguo@chromium.org,dgozman@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Committed: https://crrev.com/24e5dfb546d01394f375b61d55862201fa7df872 Cr-Commit-Position: refs/heads/master@{#40483} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/24e5dfb546d01394f375b61d55862201fa7df872 Cr-Commit-Position: refs/heads/master@{#40483} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
