Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2925023003:
DevTools: Avoid a race in WorkspaceDiff when UISourceCodes are removed (Closed)
|
Created:
3 years, 6 months ago by einbinder Modified:
3 years, 6 months ago Reviewers:
lushnikov CC:
apavlov+blink_chromium.org, blink-reviews, caseq+blink_chromium.org, chromium-reviews, devtools-reviews_chromium.org, kozyatinskiy+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, pfeldman Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionDevTools: Avoid a race in WorkspaceDiff when UISourceCodes are removed
BUG=none
Review-Url: https://codereview.chromium.org/2925023003
Cr-Commit-Position: refs/heads/master@{#478025}
Committed: https://chromium.googlesource.com/chromium/src/+/2970fc91cb6d88be096b9974bef50c0fd2c2d331
Patch Set 1 #
Total comments: 8
Patch Set 2 : done #Messages
Total messages: 14 (7 generated)
einbinder@chromium.org changed reviewers: + lushnikov@chromium.org
ptal
lgtm https://codereview.chromium.org/2925023003/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/workspace_diff/WorkspaceDiff.js (right): https://codereview.chromium.org/2925023003/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/workspace_diff/WorkspaceDiff.js:104: _removeUISourceCode(uiSourceCode) { let's jsdoc this https://codereview.chromium.org/2925023003/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/workspace_diff/WorkspaceDiff.js:179: this._invalidated = false; how about this._disposed? Feels like a better name https://codereview.chromium.org/2925023003/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/workspace_diff/WorkspaceDiff.js:222: current = await this._uiSourceCode.requestContent(); while we are here: let's add a huge // ------ ASYNC ----- comment like we do https://codereview.chromium.org/2925023003/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/workspace_diff/WorkspaceDiff.js:226: var baseline = await this._uiSourceCode.requestOriginalContent(); and let's add the "ASYNC" comment here as well
The CQ bit was checked by einbinder@chromium.org
https://codereview.chromium.org/2925023003/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/workspace_diff/WorkspaceDiff.js (right): https://codereview.chromium.org/2925023003/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/workspace_diff/WorkspaceDiff.js:104: _removeUISourceCode(uiSourceCode) { On 2017/06/08 at 00:06:51, lushnikov_ooo_june_6 wrote: > let's jsdoc this Done. https://codereview.chromium.org/2925023003/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/workspace_diff/WorkspaceDiff.js:179: this._invalidated = false; On 2017/06/08 at 00:06:51, lushnikov_ooo_june_6 wrote: > how about this._disposed? Feels like a better name Done. https://codereview.chromium.org/2925023003/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/workspace_diff/WorkspaceDiff.js:222: current = await this._uiSourceCode.requestContent(); On 2017/06/08 at 00:06:51, lushnikov_ooo_june_6 wrote: > while we are here: let's add a huge // ------ ASYNC ----- comment like we do Done. https://codereview.chromium.org/2925023003/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/workspace_diff/WorkspaceDiff.js:226: var baseline = await this._uiSourceCode.requestOriginalContent(); On 2017/06/08 at 00:06:51, lushnikov_ooo_june_6 wrote: > and let's add the "ASYNC" comment here as well Done.
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/2925023003/#ps20001 (title: "done")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by einbinder@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": 1496936718523180,
"parent_rev": "505c8510213e81599c7205e2f2698e80cf98a6c6", "commit_rev":
"2970fc91cb6d88be096b9974bef50c0fd2c2d331"}
Message was sent while issue was closed.
Description was changed from ========== DevTools: Avoid a race in WorkspaceDiff when UISourceCodes are removed BUG=none ========== to ========== DevTools: Avoid a race in WorkspaceDiff when UISourceCodes are removed BUG=none Review-Url: https://codereview.chromium.org/2925023003 Cr-Commit-Position: refs/heads/master@{#478025} Committed: https://chromium.googlesource.com/chromium/src/+/2970fc91cb6d88be096b9974bef5... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/2970fc91cb6d88be096b9974bef5... |
