[DevTools] Ship inline breakpoints (issue 2548583002 by kozyatinskiy@chromium.org)

0 views
Skip to first unread message

kozyat...@chromium.org

unread,
Dec 1, 2016, 2:34:34 PM12/1/16
to dgo...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
Reviewers: dgozman
CL: https://codereview.chromium.org/2548583002/

Message:
Dmitry, please take a look.

Description:
[DevTools] Ship inline breakpoints

Enabled inline breakpoints without experiment.
+ small UX improvement.

BUG=chromium:566801
R=dgo...@chromium.org

Affected files (+4, -6 lines):
M third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt
M third_party/WebKit/LayoutTests/inspector/sources/debugger/source-frame-inline-breakpoint-decorations.html
M third_party/WebKit/Source/devtools/front_end/main/Main.js
M third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
M third_party/WebKit/Source/devtools/front_end/text_editor/cmdevtools.css


Index: third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt
diff --git a/third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt b/third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt
index 6d3084b01246cf240f2b9e7fefeed8d963fb6143..bc0d50abc88fe5492da6500ef11bad9b9fccd462 100644
--- a/third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt
+++ b/third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt
@@ -6,7 +6,7 @@ Running: addFileSystem
Running: addNetworkFooJS

Running: setBreakpointInNetworkUISourceCode
- http://127.0.0.1:8000/inspector/persistence/resources/foo.js:0
+ http://127.0.0.1:8000/inspector/persistence/resources/foo.js:2

Running: reloadPageAndDumpBreakpoints
Page reloaded.
Index: third_party/WebKit/LayoutTests/inspector/sources/debugger/source-frame-inline-breakpoint-decorations.html
diff --git a/third_party/WebKit/LayoutTests/inspector/sources/debugger/source-frame-inline-breakpoint-decorations.html b/third_party/WebKit/LayoutTests/inspector/sources/debugger/source-frame-inline-breakpoint-decorations.html
index 93329e48bed3a3fb434790836025e8dbfded7421..92a0e63c7fc77bab3424e46b111d987a081d1e7d 100644
--- a/third_party/WebKit/LayoutTests/inspector/sources/debugger/source-frame-inline-breakpoint-decorations.html
+++ b/third_party/WebKit/LayoutTests/inspector/sources/debugger/source-frame-inline-breakpoint-decorations.html
@@ -21,7 +21,6 @@ function test()
.then(() => InspectorTest.dumpJavaScriptSourceFrameBreakpoints(sourceFrame));
}

- Runtime.experiments.enableForTest("inlineBreakpoints");
Bindings.breakpointManager._storage._breakpoints = {};
InspectorTest.runDebuggerTestSuite([
function testAddRemoveBreakpoint(next)
Index: third_party/WebKit/Source/devtools/front_end/main/Main.js
diff --git a/third_party/WebKit/Source/devtools/front_end/main/Main.js b/third_party/WebKit/Source/devtools/front_end/main/Main.js
index b313e24603938e75ddc198b6c3e263c6d23b5777..b8a510aa01461248cb20a5da8bb23500ba6ca77c 100644
--- a/third_party/WebKit/Source/devtools/front_end/main/Main.js
+++ b/third_party/WebKit/Source/devtools/front_end/main/Main.js
@@ -111,7 +111,6 @@ Main.Main = class {
Runtime.experiments.register('resolveVariableNames', 'Resolve variable names');
Runtime.experiments.register('timelineShowAllEvents', 'Show all events on Timeline', true);
Runtime.experiments.register('timelineShowAllProcesses', 'Show all processes on Timeline', true);
- Runtime.experiments.register('inlineBreakpoints', 'Show inline breakpoints');
Runtime.experiments.register('securityPanel', 'Security panel');
Runtime.experiments.register('sourceDiff', 'Source diff');
Runtime.experiments.register('terminalInDrawer', 'Terminal in drawer', true);
Index: third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
diff --git a/third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js b/third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
index 628f2ab47b7b24218a0dbb9c3052418c9ed78755..f0fb003b1244338978013565618bf8158d3486d8 100644
--- a/third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
+++ b/third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
@@ -916,7 +916,7 @@ Sources.JavaScriptSourceFrame = class extends Sources.UISourceCodeFrame {
}
breakpoint[Sources.JavaScriptSourceFrame.BreakpointDecoration._decorationSymbol] = decoration;
this._updateBreakpointDecoration(decoration);
- if (!lineDecorations.length && Runtime.experiments.isEnabled('inlineBreakpoints')) {
+ if (!lineDecorations.length) {
this._willAddInlineDecorationsForTest();
this._breakpointManager
.possibleBreakpoints(
@@ -1307,7 +1307,7 @@ Sources.JavaScriptSourceFrame.BreakpointDecoration = class {
}

show() {
- if (this.bookmark || !Runtime.experiments.isEnabled('inlineBreakpoints'))
+ if (this.bookmark)
return;
var location = this.handle.resolve();
if (!location)
Index: third_party/WebKit/Source/devtools/front_end/text_editor/cmdevtools.css
diff --git a/third_party/WebKit/Source/devtools/front_end/text_editor/cmdevtools.css b/third_party/WebKit/Source/devtools/front_end/text_editor/cmdevtools.css
index 18590e195e5baeae45d475609d64fabd93cc1bd1..969daea5acdbf78529e86030d050ab322b6e296d 100644
--- a/third_party/WebKit/Source/devtools/front_end/text_editor/cmdevtools.css
+++ b/third_party/WebKit/Source/devtools/front_end/text_editor/cmdevtools.css
@@ -121,7 +121,7 @@

.cm-inline-breakpoint {
position:relative;
- top: 1px;
+ top: 2px;
cursor: pointer;
}



dgo...@chromium.org

unread,
Dec 1, 2016, 8:37:48 PM12/1/16
to kozyat...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
lgtm




https://codereview.chromium.org/2548583002/diff/20001/third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt
File
third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt
(right):

https://codereview.chromium.org/2548583002/diff/20001/third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt#newcode13
third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt:13:
file:///var/www/inspector/persistence/resources/foo.js:0
Let's fix the test to wait for resolving here.

https://codereview.chromium.org/2548583002/

kozyat...@chromium.org

unread,
Dec 1, 2016, 9:35:11 PM12/1/16
to dgo...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org

https://codereview.chromium.org/2548583002/diff/20001/third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt
File
third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt
(right):

https://codereview.chromium.org/2548583002/diff/20001/third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt#newcode13
third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-move-breakpoints-on-reload-expected.txt:13:
file:///var/www/inspector/persistence/resources/foo.js:0
On 2016/12/02 01:37:48, dgozman wrote:
> Let's fix the test to wait for resolving here.

This dump 0 here because we persist breakpoint on location where they
was set - not resolved and after page reloaded when script isn't exist
we can't resolve it into better location.

https://codereview.chromium.org/2548583002/

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 1, 2016, 9:35:37 PM12/1/16
to kozyat...@chromium.org, dgo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 2, 2016, 12:25:58 AM12/2/16
to kozyat...@chromium.org, dgo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@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_ng/builds/342578)

https://codereview.chromium.org/2548583002/

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 2, 2016, 1:17:07 PM12/2/16
to kozyat...@chromium.org, dgo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 2, 2016, 3:23:48 PM12/2/16
to kozyat...@chromium.org, dgo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 2, 2016, 5:10:45 PM12/2/16
to kozyat...@chromium.org, dgo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
Committed patchset #3 (id:40001)

https://codereview.chromium.org/2548583002/

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 2, 2016, 5:14:28 PM12/2/16
to kozyat...@chromium.org, dgo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
Patchset 3 (id:??) landed as
https://crrev.com/225f2381b9e5184fc8edd59baacca0f1d74d7158
Cr-Commit-Position: refs/heads/master@{#436043}

https://codereview.chromium.org/2548583002/
Reply all
Reply to author
Forward
0 new messages