Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2536033003:
Clarify thread safety of WebAudioSourceProvider interface. (Closed)
|
Created:
4 years ago by DaleCurtis Modified:
4 years ago CC:
blink-reviews, blink-reviews-api_chromium.org, chromium-reviews, dglazkov+blink, o1ka Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionClarify thread safety of WebAudioSourceProvider interface.
Update documentation to reflect current usage.
BUG=615589, 647498
TEST=none
Committed: https://crrev.com/1e29e41f663761f2447030dbdf7e45ea9179ed7a
Cr-Commit-Position: refs/heads/master@{#435090}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Comments. #
Total comments: 2
Patch Set 3 : Fix nits. #Messages
Total messages: 15 (5 generated)
dalecurtis@chromium.org changed reviewers: + foolip@chromium.org, rtoy@chromium.org
https://codereview.chromium.org/2536033003/diff/1/third_party/WebKit/public/p... File third_party/WebKit/public/platform/WebAudioSourceProvider.h (right): https://codereview.chromium.org/2536033003/diff/1/third_party/WebKit/public/p... third_party/WebKit/public/platform/WebAudioSourceProvider.h:45: // Must always be called from the same thread. "same thread" as what?
https://codereview.chromium.org/2536033003/diff/1/third_party/WebKit/public/p... File third_party/WebKit/public/platform/WebAudioSourceProvider.h (right): https://codereview.chromium.org/2536033003/diff/1/third_party/WebKit/public/p... third_party/WebKit/public/platform/WebAudioSourceProvider.h:45: // Must always be called from the same thread. On 2016/11/29 at 19:17:12, Raymond Toy wrote: > "same thread" as what? I.e. if you're on t0 and call setClient(), all future calls must be issued on t0 as well. Is there language you would prefer?
lgtm https://codereview.chromium.org/2536033003/diff/1/third_party/WebKit/public/p... File third_party/WebKit/public/platform/WebAudioSourceProvider.h (right): https://codereview.chromium.org/2536033003/diff/1/third_party/WebKit/public/p... third_party/WebKit/public/platform/WebAudioSourceProvider.h:45: // Must always be called from the same thread. On 2016/11/29 19:21:40, DaleCurtis wrote: > On 2016/11/29 at 19:17:12, Raymond Toy wrote: > > "same thread" as what? > > I.e. if you're on t0 and call setClient(), all future calls must be issued on t0 > as well. Is there language you would prefer? I think that's a perfect comment. "All future calls to setClient must be issued from the same thread as the first call to setClient". Or something like that.
https://codereview.chromium.org/2536033003/diff/1/third_party/WebKit/public/p... File third_party/WebKit/public/platform/WebAudioSourceProvider.h (right): https://codereview.chromium.org/2536033003/diff/1/third_party/WebKit/public/p... third_party/WebKit/public/platform/WebAudioSourceProvider.h:45: // Must always be called from the same thread. On 2016/11/29 at 19:23:50, Raymond Toy wrote: > On 2016/11/29 19:21:40, DaleCurtis wrote: > > On 2016/11/29 at 19:17:12, Raymond Toy wrote: > > > "same thread" as what? > > > > I.e. if you're on t0 and call setClient(), all future calls must be issued on t0 > > as well. Is there language you would prefer? > > I think that's a perfect comment. "All future calls to setClient must be issued from the same thread as the first call to setClient". Or something like that. Done.
lgtm % once https://codereview.chromium.org/2536033003/diff/20001/third_party/WebKit/publ... File third_party/WebKit/public/platform/WebAudioSourceProvider.h (right): https://codereview.chromium.org/2536033003/diff/20001/third_party/WebKit/publ... third_party/WebKit/public/platform/WebAudioSourceProvider.h:45: // Must always be called from the same thread. I.e., Once called on a thread, s/Once/once/
The CQ bit was checked by dalecurtis@chromium.org
Thanks for review! https://codereview.chromium.org/2536033003/diff/20001/third_party/WebKit/publ... File third_party/WebKit/public/platform/WebAudioSourceProvider.h (right): https://codereview.chromium.org/2536033003/diff/20001/third_party/WebKit/publ... third_party/WebKit/public/platform/WebAudioSourceProvider.h:45: // Must always be called from the same thread. I.e., Once called on a thread, On 2016/11/29 at 20:09:44, foolip wrote: > s/Once/once/ Done.
The patchset sent to the CQ was uploaded after l-g-t-m from rtoy@chromium.org, foolip@chromium.org Link to the patchset: https://codereview.chromium.org/2536033003/#ps40001 (title: "Fix nits.")
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": 1480450668807030,
"parent_rev": "ac7a450b53ffcb51bb47cc21f4021234c96af540", "commit_rev":
"b912cd8e965735fd0f8aa8b4f4d778c1dabc26b4"}
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Clarify thread safety of WebAudioSourceProvider interface. Update documentation to reflect current usage. BUG=615589,647498 TEST=none ========== to ========== Clarify thread safety of WebAudioSourceProvider interface. Update documentation to reflect current usage. BUG=615589,647498 TEST=none Committed: https://crrev.com/1e29e41f663761f2447030dbdf7e45ea9179ed7a Cr-Commit-Position: refs/heads/master@{#435090} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/1e29e41f663761f2447030dbdf7e45ea9179ed7a Cr-Commit-Position: refs/heads/master@{#435090} |
