Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2546583003:
DevTools: [Persistence] guard against multiple UISourceCodes with similar URL (Closed)
|
Created:
4 years ago by lushnikov Modified:
4 years ago Reviewers:
dgozman 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. |
DescriptionDevTools: [Persistence] guard against multiple UISourceCodes with similar URL
Currently, we assume that there is a single UISourceCode per URL in the
Workspace.
However, this is not the case. In order to avoid unpredicted behavior,
we should guard against this sitation.
BUG=649837
R=dgozman
Committed: https://crrev.com/7ac7d36de53c8632613f193adcafa9d1678e4e39
Cr-Commit-Position: refs/heads/master@{#436486}
Patch Set 1 #Patch Set 2 : add fixme #
Total comments: 3
Patch Set 3 : put check deep down #
Messages
Total messages: 24 (16 generated)
please, take a look
The CQ bit was checked by lushnikov@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...
The CQ bit was checked by lushnikov@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2546583003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js (right): https://codereview.chromium.org/2546583003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js:35: // FIXME(lushnikov): workspace should contain only one UISourceCode per URL. TODO https://codereview.chromium.org/2546583003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js:36: if (binding.network[Persistence.Persistence._binding] || binding.fileSystem[Persistence.Persistence._binding]) I think we should move this check to both mappings instead.
https://codereview.chromium.org/2546583003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js (right): https://codereview.chromium.org/2546583003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js:36: if (binding.network[Persistence.Persistence._binding] || binding.fileSystem[Persistence.Persistence._binding]) On 2016/12/01 18:29:06, dgozman wrote: > I think we should move this check to both mappings instead. Since Persistence instance is nicely isolated from mappings - this feels like a good place by itself. wdyt?
addressed comments. please, take a look
The CQ bit was checked by lushnikov@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
The CQ bit was checked by lushnikov@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
The CQ bit was unchecked by lushnikov@chromium.org
The CQ bit was checked by lushnikov@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": 40001, "attempt_start_ts": 1480981672707910,
"parent_rev": "236445a2e131b6cae0e5454242eff56605052396", "commit_rev":
"6b5f83521a22f50d474d7aac3d1bb4511cf7189c"}
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== DevTools: [Persistence] guard against multiple UISourceCodes with similar URL Currently, we assume that there is a single UISourceCode per URL in the Workspace. However, this is not the case. In order to avoid unpredicted behavior, we should guard against this sitation. BUG=649837 R=dgozman ========== to ========== DevTools: [Persistence] guard against multiple UISourceCodes with similar URL Currently, we assume that there is a single UISourceCode per URL in the Workspace. However, this is not the case. In order to avoid unpredicted behavior, we should guard against this sitation. BUG=649837 R=dgozman Committed: https://crrev.com/7ac7d36de53c8632613f193adcafa9d1678e4e39 Cr-Commit-Position: refs/heads/master@{#436486} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/7ac7d36de53c8632613f193adcafa9d1678e4e39 Cr-Commit-Position: refs/heads/master@{#436486} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
