Reland "[inspector] breakpoint after last break position should not jump to first line"

alexkozy · Commit Bot · commit b1827e9bc351 · 2017-10-16T22:04:55.000Z
This is a reland of 61292f0 Original change's description: > [inspector] breakpoint after last break position should not jump to first line > > R=jgruber@chromium.org > > Bug: chromium:730177 > Change-Id: I0f3666a333604cb80bb51410c5edf2aceb0c6ef5 > Reviewed-on: https://chromium-review.googlesource.com/717717 > Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> > Reviewed-by: Jakob Gruber <jgruber@chromium.org> > Cr-Commit-Position: refs/heads/master@{#48556} TBR=jgruber@chromium.org Bug: chromium:730177 Change-Id: I564cc5d7778f9d79780eae9dbe2d9aafaad4f466 Reviewed-on: https://chromium-review.googlesource.com/721468 Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> Cr-Commit-Position: refs/heads/master@{#48615}
diff --git a/src/debug/debug.cc b/src/debug/debug.cc
@@ -603,8 +603,13 @@ bool Debug::SetBreakPointForScript(Handle<Script> script,
 
   Handle<DebugInfo> debug_info(shared->GetDebugInfo());
 
-  // Find the break point and change it.
-  *source_position = FindBreakablePosition(debug_info, *source_position);
+  // Find breakable position returns first breakable position after
+  // *source_position, it can return 0 if no break location is found after
+  // *source_position.
+  int breakable_position = FindBreakablePosition(debug_info, *source_position);
+  if (breakable_position < *source_position) return false;
+  *source_position = breakable_position;
+
   DebugInfo::SetBreakPoint(debug_info, *source_position, break_point_object);
   // At least one active break point now.
   DCHECK_LT(0, debug_info->GetBreakPointCount());
diff --git a/test/debugger/debug/regress/regress-crbug-517592.js b/test/debugger/debug/regress/regress-crbug-517592.js
@@ -4,7 +4,7 @@
 
 var source =
   "var foo = function foo() {\n" +
-  "  return 1;\n" +
+  "  var a = 1;\n" +
   "}\n" +
   "//@ sourceURL=test";
 
diff --git a/test/inspector/debugger/set-breakpoint-at-last-line-expected.txt b/test/inspector/debugger/set-breakpoint-at-last-line-expected.txt
@@ -0,0 +1,16 @@
+Tests breakpoint at last line.
+{
+    breakpointId : <breakpointId>
+    locations : [
+        [0] : {
+            columnNumber : 12
+            lineNumber : 3
+            scriptId : <scriptId>
+        }
+    ]
+}
+{
+    breakpointId : <breakpointId>
+    locations : [
+    ]
+}
diff --git a/test/inspector/debugger/set-breakpoint-at-last-line.js b/test/inspector/debugger/set-breakpoint-at-last-line.js
@@ -0,0 +1,28 @@
+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+let {session, contextGroup, Protocol} = InspectorTest.start('Tests breakpoint at last line.');
+
+let source = `
+  let a = 1;
+  //# sourceURL=foo.js
+  let b = 2;
+`;
+
+(async function test() {
+  Protocol.Debugger.enable();
+  Protocol.Runtime.evaluate({expression: source});
+  let {result} = await Protocol.Debugger.setBreakpointByUrl({
+    url: 'foo.js',
+    lineNumber: 3,
+    columnNumber: 12
+  });
+  InspectorTest.logMessage(result);
+  ({result} = await Protocol.Debugger.setBreakpointByUrl({
+    url: 'foo.js',
+    lineNumber: 4
+  }));
+  InspectorTest.logMessage(result);
+  InspectorTest.completeTest();
+})();
