[DevTools] Prepare JavaScriptSourceFrame for multiple breakpoints per line

[DevTools] Prepare JavaScriptSourceFrame for multiple breakpoints per line

- used textEditorPositionHandle for breakpoint location tracking during editing,
- prepared context menu and line decorations for multiple breakpoints in one line.

BUG=chromium:566801
R=lushnikov@chromium.org,dgozman@chromium.org

Committed: https://crrev.com/979c4d82518fda1ea8182a7368da152b6bd38088
Cr-Commit-Position: refs/heads/master@{#433655}

[kozy, 2016-11-17 18:23:56]

Andrey, please take a look.
I extracted from https://codereview.chromium.org/2500493003 part which prepares
JavaScriptSourceFrame for new world with more then one breakpoint per line.

Roadmap:
- use textEditorPositionHandle to track moving locations after edit,
- introduce inline breakpoints.

[kozy, 2016-11-17 18:25:28]

Description was changed from

==========
[DevTools] Prepare JavaScriptSourceFrame for multiple breakpoints per line

- replaced existing breakpoint decoration mutation methods with
updateBreakpointDecorations.
New method is scheduled when state of decorations is changed and then at the end
of microtask it will update breakpoint decorations for affected lines in single
CodeMirror operation.
- JavaScriptSourceFrame manage line classes for breakpoints by itself.
- decorations and context menu are ready for multiple breakpoints per line.

BUG=chromium:566801
R=lushnikov@chromium.org,dgozman@chromium.org
==========

to

==========
[DevTools] Prepare JavaScriptSourceFrame for multiple breakpoints per line

- replaced existing breakpoint decoration mutation methods with
updateBreakpointDecorations.
New method is scheduled when state of decorations is changed and then at the end
of microtask it will update breakpoint decorations for affected lines in single
CodeMirror operation.
- JavaScriptSourceFrame manages line classes for breakpoints by itself.
- decorations and context menu are ready for multiple breakpoints per line.

BUG=chromium:566801
R=lushnikov@chromium.org,dgozman@chromium.org
==========

[lushnikov, 2016-11-17 21:33:18]

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
(right):

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:372:
for (var breakpointLocation of
this._breakpointManager.breakpointLocationsForUISourceCode(this.uiSourceCode()))
i don't think it returns provisional breakpoints. Will this work for FS source
codes?

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:415:
for (var breakpointLocation of
this._breakpointManager.breakpointLocationsForUISourceCode(this.uiSourceCode()))
{
same question here about file system UISourceCodes

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:599:
this.textEditor.toggleLineClass(lineNumber, 'cm-breakpoint-disabled', hasAny &&
(!hasEnabled && !this._muted));
what is this._muted flag? do we really need it?

[lushnikov, 2016-11-17 22:14:53]

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
(left):

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:408:
for (var lineNumber = 0; lineNumber < this._textEditor.linesCount; ++lineNumber)
{
unfortunately we need to traverse lines, because editor moves breakpoint
decorations as the edits happen

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
(right):

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:589:
for (var breakpoint of breakpoints) {
breakpoints.sort((b1, b2) => {
  if (!!b1.condition() !== !!b2.condition())
    return b1.condition() ? -1 : 1;
  if (b1.enabled() !== b2.enabled())
    return b1.enabled() ? -1 : 1;
  return 0;
});

var leadBreakpoint = breakpoints[0];
this.textEditor.toggleLineClas(lineNumber, 'cm-breakpoint', true);
....

[kozy, 2016-11-18 16:28:14]

Description was changed from

==========
[DevTools] Prepare JavaScriptSourceFrame for multiple breakpoints per line

- replaced existing breakpoint decoration mutation methods with
updateBreakpointDecorations.
New method is scheduled when state of decorations is changed and then at the end
of microtask it will update breakpoint decorations for affected lines in single
CodeMirror operation.
- JavaScriptSourceFrame manages line classes for breakpoints by itself.
- decorations and context menu are ready for multiple breakpoints per line.

BUG=chromium:566801
R=lushnikov@chromium.org,dgozman@chromium.org
==========

to

==========
[DevTools] Prepare JavaScriptSourceFrame for multiple breakpoints per line

- used textEditorPositionHandle for breakpoint location tracking during editing,
- prepared context menu and line decorations for multiple breakpoints in one
line.

BUG=chromium:566801
R=lushnikov@chromium.org,dgozman@chromium.org
==========

[kozy, 2016-11-18 16:29:59]

Patchset #2 (id:20001) has been deleted

[kozy, 2016-11-18 16:32:40]

I used textEditorPositionHandle to track breakpoint locations during editing and
addressed all comments. Please take another look.
I'll improve live-edit-breakpoint test soon.

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
(left):

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:408:
for (var lineNumber = 0; lineNumber < this._textEditor.linesCount; ++lineNumber)
{
On 2016/11/17 22:14:52, lushnikov wrote:
> unfortunately we need to traverse lines, because editor moves breakpoint
> decorations as the edits happen

Done.

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
(right):

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:372:
for (var breakpointLocation of
this._breakpointManager.breakpointLocationsForUISourceCode(this.uiSourceCode()))
On 2016/11/17 21:33:18, lushnikov wrote:
> i don't think it returns provisional breakpoints. Will this work for FS source
> codes?

It returns.

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:415:
for (var breakpointLocation of
this._breakpointManager.breakpointLocationsForUISourceCode(this.uiSourceCode()))
{
On 2016/11/17 21:33:18, lushnikov wrote:
> same question here about file system UISourceCodes

returns.

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:589:
for (var breakpoint of breakpoints) {
On 2016/11/17 22:14:52, lushnikov wrote:
> breakpoints.sort((b1, b2) => {
>   if (!!b1.condition() !== !!b2.condition())
>     return b1.condition() ? -1 : 1;
>   if (b1.enabled() !== b2.enabled())
>     return b1.enabled() ? -1 : 1;
>   return 0;
> });
> 
> var leadBreakpoint = breakpoints[0];
> this.textEditor.toggleLineClas(lineNumber, 'cm-breakpoint', true);
> ....

Done.

https://codereview.chromium.org/2505413002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:599:
this.textEditor.toggleLineClass(lineNumber, 'cm-breakpoint-disabled', hasAny &&
(!hasEnabled && !this._muted));
On 2016/11/17 21:33:18, lushnikov wrote:
> what is this._muted flag? do we really need it? 

We can remove it later with better understanding why we need separate handling
for snippets.

[lushnikov, 2016-11-18 21:44:46]

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/http/tests/inspector/debugger-test.js
(right):

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/debugger-test.js:630:
InspectorTest.waitJavaScriptSourceFrameBreakpoints = function(sourceFrame)
let's wait for amount of resolved breakpoints in breakpoint manager.

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/source-frame-expected.txt
(left):

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/source-frame-expected.txt:7:
Breakpoint added to source frame at line 14
where did it go? Do we really this test?

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/source-frame.html
(right):

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/source-frame.html:34:
function didAddBreakpoint(lineNumber)
you don't use lineNumber

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
(right):

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:415:
.map(decoration => decoration.serialize())
let's inline serialize here

or you may even use breakpoint straightaway!..

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:817:
this._scheduledBreakpointDecorationUpdates = new Set();
you should put decorations instead of lines inside the set

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:893:
this._breakpointDecorations.delete(decoration);
cleanup symbol as well
breakpoint[Sources.JavaScriptSourceFrame.BreakpointDecoration._decorationSymbol];

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:1044:
var hasDisabled = decorations.some(decoration => !decoration.enabled);
can we check for cm-breakpoint-disabled class on gutter instead?

[kozy, 2016-11-19 00:46:09]

all done, please take a look!

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/http/tests/inspector/debugger-test.js
(right):

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/debugger-test.js:630:
InspectorTest.waitJavaScriptSourceFrameBreakpoints = function(sourceFrame)
On 2016/11/18 21:44:46, lushnikov wrote:
> let's wait for amount of resolved breakpoints in breakpoint manager.

Done.

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/source-frame-expected.txt
(left):

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/source-frame-expected.txt:7:
Breakpoint added to source frame at line 14
On 2016/11/18 21:44:46, lushnikov wrote:
> where did it go? Do we really this test?

removed.

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/source-frame.html
(right):

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/source-frame.html:34:
function didAddBreakpoint(lineNumber)
On 2016/11/18 21:44:46, lushnikov wrote:
> you don't use lineNumber

removed, more robust test was added.

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
(right):

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:415:
.map(decoration => decoration.serialize())
On 2016/11/18 21:44:46, lushnikov wrote:
> let's inline serialize here
> 
> or you may even use breakpoint straightaway!..

Done.

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:817:
this._scheduledBreakpointDecorationUpdates = new Set();
On 2016/11/18 21:44:46, lushnikov wrote:
> you should put decorations instead of lines inside the set

Done.

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:893:
this._breakpointDecorations.delete(decoration);
On 2016/11/18 21:44:46, lushnikov wrote:
> cleanup symbol as well
>
breakpoint[Sources.JavaScriptSourceFrame.BreakpointDecoration._decorationSymbol];

Done.

https://codereview.chromium.org/2505413002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:1044:
var hasDisabled = decorations.some(decoration => !decoration.enabled);
On 2016/11/18 21:44:46, lushnikov wrote:
> can we check for cm-breakpoint-disabled class on gutter instead?

Done.

[lushnikov, 2016-11-21 19:17:23]

lgtm

https://codereview.chromium.org/2505413002/diff/140001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
(right):

https://codereview.chromium.org/2505413002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:810:
this._scheduledBreakpointDecorationUpdates = new Set();
nit: jsdoc

https://codereview.chromium.org/2505413002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:814:
/**
newline

[kozy, 2016-11-21 19:40:10]

all done! thanks a lot!

https://codereview.chromium.org/2505413002/diff/140001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
(right):

https://codereview.chromium.org/2505413002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:810:
this._scheduledBreakpointDecorationUpdates = new Set();
On 2016/11/21 19:17:23, lushnikov wrote:
> nit: jsdoc

Done.

https://codereview.chromium.org/2505413002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:814:
/**
On 2016/11/21 19:17:23, lushnikov wrote:
> newline

Done.

[kozy, 2016-11-21 19:40:14]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2016-11-21 19:40:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
lushnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/2505413002/#ps160001
(title: "addressed comments")

[commit-bot: I haz the power, 2016-11-21 19:40:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-21 21:20:57]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1479757214266020,
"parent_rev": "7db32fe3b1bee1832e1923559de9099b2d3677d9", "commit_rev":
"e9818538bf66963ad926bee165c9e79d598ed4b3"}

[commit-bot: I haz the power, 2016-11-21 21:21:18]

Description was changed from

==========
[DevTools] Prepare JavaScriptSourceFrame for multiple breakpoints per line

- used textEditorPositionHandle for breakpoint location tracking during editing,
- prepared context menu and line decorations for multiple breakpoints in one
line.

BUG=chromium:566801
R=lushnikov@chromium.org,dgozman@chromium.org
==========

to

==========
[DevTools] Prepare JavaScriptSourceFrame for multiple breakpoints per line

- used textEditorPositionHandle for breakpoint location tracking during editing,
- prepared context menu and line decorations for multiple breakpoints in one
line.

BUG=chromium:566801
R=lushnikov@chromium.org,dgozman@chromium.org
==========

[commit-bot: I haz the power, 2016-11-21 21:21:19]

Committed patchset #8 (id:160001)

[commit-bot: I haz the power, 2016-11-21 21:27:52]

Description was changed from

==========
[DevTools] Prepare JavaScriptSourceFrame for multiple breakpoints per line

- used textEditorPositionHandle for breakpoint location tracking during editing,
- prepared context menu and line decorations for multiple breakpoints in one
line.

BUG=chromium:566801
R=lushnikov@chromium.org,dgozman@chromium.org
==========

to

==========
[DevTools] Prepare JavaScriptSourceFrame for multiple breakpoints per line

- used textEditorPositionHandle for breakpoint location tracking during editing,
- prepared context menu and line decorations for multiple breakpoints in one
line.

BUG=chromium:566801
R=lushnikov@chromium.org,dgozman@chromium.org

Committed: https://crrev.com/979c4d82518fda1ea8182a7368da152b6bd38088
Cr-Commit-Position: refs/heads/master@{#433655}
==========

[commit-bot: I haz the power, 2016-11-21 21:27:53]

Patchset 8 (id:??) landed as
https://crrev.com/979c4d82518fda1ea8182a7368da152b6bd38088
Cr-Commit-Position: refs/heads/master@{#433655}