Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2239753003:
DevTools: Make sure worker instance is not collected. (Closed)
|
Created:
4 years, 4 months ago by alph Modified:
4 years, 4 months ago 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, kozyatinskiy+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDevTools: Make sure the worker instance is not collected while the promise is in flight.
BUG=637161
Committed: https://crrev.com/e68f462e0eee7ce6a4befad62fee5fb76bc61da4
Cr-Commit-Position: refs/heads/master@{#411553}
Patch Set 1 #Patch Set 2 : addressing comment. #Messages
Total messages: 18 (9 generated)
alph@chromium.org changed reviewers: + lushnikov@chromium.org, pfeldman@chromium.org
Description was changed from ========== DevTools: Make sure worker instance did not get collected. BUG=637161 ========== to ========== DevTools: Make sure worker instance is not collected. BUG=637161 ==========
The CQ bit was checked by pfeldman@chromium.org
lgtm
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 pfeldman@chromium.org
We should not have a _worker and _workerPromise on the instance at a time. It should be _workerProtect and you should clear it upon resolution.
this is so good. just incredible.
Description was changed from ========== DevTools: Make sure worker instance is not collected. BUG=637161 ========== to ========== DevTools: Make sure the worker instance is not collected while the promise is in flight. BUG=637161 ==========
On 2016/08/12 02:00:55, pfeldman wrote: > We should not have a _worker and _workerPromise on the instance at a time. It > should be _workerProtect and you should clear it upon resolution. Done. Though it needs refactoring anyways. The simple version is just for merging.
The CQ bit was checked by alph@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from pfeldman@chromium.org Link to the patchset: https://codereview.chromium.org/2239753003/#ps20001 (title: "addressing comment.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== DevTools: Make sure the worker instance is not collected while the promise is in flight. BUG=637161 ========== to ========== DevTools: Make sure the worker instance is not collected while the promise is in flight. BUG=637161 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== DevTools: Make sure the worker instance is not collected while the promise is in flight. BUG=637161 ========== to ========== DevTools: Make sure the worker instance is not collected while the promise is in flight. BUG=637161 Committed: https://crrev.com/e68f462e0eee7ce6a4befad62fee5fb76bc61da4 Cr-Commit-Position: refs/heads/master@{#411553} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/e68f462e0eee7ce6a4befad62fee5fb76bc61da4 Cr-Commit-Position: refs/heads/master@{#411553} |
