Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2552323003:
[DevTools] clean breakpoints and decorations onUISourceCodeContentChanged (Closed)
|
Created:
4 years ago by kozy Modified:
4 years ago Reviewers:
lushnikov CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, pfeldman, kozyatinskiy+blink_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[DevTools] clean breakpoints and decorations onUISourceCodeContentChanged
BUG=chromium:671924
R=lushnikov@chromium.org
Committed: https://crrev.com/8632faa297c779a896d878a5126b58f679514158
Cr-Commit-Position: refs/heads/master@{#436888}
Patch Set 1 #
Total comments: 2
Patch Set 2 : better #Messages
Total messages: 18 (11 generated)
Andrey, please take a look.
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...
https://codereview.chromium.org/2552323003/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js (right): https://codereview.chromium.org/2552323003/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:189: this._textEditor.operation(() => decorations.map(decoration => decoration.hide())); Why do we need "hide"? Can't we have just this: for (var decoration of this._breakpointDecorations.valuesArray()) { this._breakpointDecorations.delete(decoration); this._updateBreakpointDecoration(decoration); if (decoration.breakpoint) decoration.breakpoint.remove(); }
all done, please take another look! https://codereview.chromium.org/2552323003/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js (right): https://codereview.chromium.org/2552323003/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js:189: this._textEditor.operation(() => decorations.map(decoration => decoration.hide())); On 2016/12/07 05:08:40, lushnikov wrote: > Why do we need "hide"? Can't we have just this: > > for (var decoration of this._breakpointDecorations.valuesArray()) { > this._breakpointDecorations.delete(decoration); > this._updateBreakpointDecoration(decoration); > if (decoration.breakpoint) > decoration.breakpoint.remove(); > } You're right. breakpoint.remove will synchronously remove decoration and sync its update. In update procedures I'll remove all inline decorations since there is no real breakpoint in line.
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...
lgtm let's mention in the issue description that we're actually fixing an exception
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== [DevTools] clean breakpoints and decorations onUISourceCodeContentChanged BUG=none R=lushnikov@chromium.org ========== to ========== [DevTools] clean breakpoints and decorations onUISourceCodeContentChanged BUG=chromium:671924 R=lushnikov@chromium.org ==========
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": 20001, "attempt_start_ts": 1481095669561210,
"parent_rev": "f5794bca7595ebb7176c3639065032975d432dc2", "commit_rev":
"009142190cfd6c3ea118a5e056707f430fa9fa43"}
Message was sent while issue was closed.
Description was changed from ========== [DevTools] clean breakpoints and decorations onUISourceCodeContentChanged BUG=chromium:671924 R=lushnikov@chromium.org ========== to ========== [DevTools] clean breakpoints and decorations onUISourceCodeContentChanged BUG=chromium:671924 R=lushnikov@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [DevTools] clean breakpoints and decorations onUISourceCodeContentChanged BUG=chromium:671924 R=lushnikov@chromium.org ========== to ========== [DevTools] clean breakpoints and decorations onUISourceCodeContentChanged BUG=chromium:671924 R=lushnikov@chromium.org Committed: https://crrev.com/8632faa297c779a896d878a5126b58f679514158 Cr-Commit-Position: refs/heads/master@{#436888} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/8632faa297c779a896d878a5126b58f679514158 Cr-Commit-Position: refs/heads/master@{#436888} |
