Initialize DeferredTaskHandler's audio thread ID once.

Initialize DeferredTaskHandler's audio thread ID once.

Avoid repeatedly recording the audio thread's ID, along with assertedly
verifying that this is done off the main thread.

R=
BUG=590108
Committed: https://crrev.com/cd465f7624dfd687c7506028a5c0553aa3e4cd1b
Cr-Commit-Position: refs/heads/master@{#391848}

[sof, 2016-05-05 07:56:34]

Description was changed from

==========
Initialize DeferredTaskHandler's audio thread once.

Avoid repeatedly recording the audio thread's ID, along with assertedly
verifying that this is done off the main thread.

R=
BUG=590108
==========

to

==========
Initialize DeferredTaskHandler's audio thread ID once.

Avoid repeatedly recording the audio thread's ID, along with assertedly
verifying that this is done off the main thread.

R=
BUG=590108
==========

[sof, 2016-05-05 08:02:03]

sigbjornf@opera.com changed reviewers:
+ haraken@chromium.org, hongchan@chromium.org, rtoy@chromium.org

[sof, 2016-05-05 08:02:04]

please take a look.

Maybe this is being too defensive, checking that the main thread isn't recorded
as the audio thread, but it would take one potential issue off the table for the
associated bug.

[haraken, 2016-05-05 10:12:43]

LGTM

[sof, 2016-05-05 14:58:35]

thanks, will let this wait overnight on webaudio owner feedback

[Raymond Toy, 2016-05-05 15:58:49]

lgtm with a few nits

https://codereview.chromium.org/1954663002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h (right):

https://codereview.chromium.org/1954663002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h:102: void
setCurrentAsAudioThread()
I have a slight preference for setAudioThreadToCurrentThread.

https://codereview.chromium.org/1954663002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h:106:
ASSERT(m_audioThread == currentThread());
Isn't it DCHECK() instead of ASSERT() now?

https://codereview.chromium.org/1954663002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h:107: return;
I don't think it's possible for the assert to fail, but what should happen if
the threads don't actually match?

[sof, 2016-05-05 16:31:43]

https://codereview.chromium.org/1954663002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h (right):

https://codereview.chromium.org/1954663002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h:102: void
setCurrentAsAudioThread()
On 2016/05/05 15:58:49, Raymond Toy wrote:
> I have a slight preference for setAudioThreadToCurrentThread.

renamed

https://codereview.chromium.org/1954663002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h:106:
ASSERT(m_audioThread == currentThread());
On 2016/05/05 15:58:49, Raymond Toy wrote:
> Isn't it DCHECK() instead of ASSERT() now?

The current story is clouded; I prefer consistency per file & module, and
ASSERT() is what's used.

https://codereview.chromium.org/1954663002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h:107: return;
On 2016/05/05 15:58:49, Raymond Toy wrote:
> I don't think it's possible for the assert to fail, but what should happen if
> the threads don't actually match?

isAudioThread() isn't prepared for that, so it should fail. An unlikely case
best handled via a (non-release) assert, was how I reasoned.

[hongchan, 2016-05-05 16:39:45]

lgtm with one nit.

https://codereview.chromium.org/1954663002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h (right):

https://codereview.chromium.org/1954663002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h:111: }
NIT: I don't know what the convention is, but this method has multiple lines -
so perhaps better to have the definition in .cpp file?

[sof, 2016-05-05 16:46:31]

https://codereview.chromium.org/1954663002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h (right):

https://codereview.chromium.org/1954663002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h:111: }
On 2016/05/05 16:39:45, hoch wrote:
> NIT: I don't know what the convention is, but this method has multiple lines -
> so perhaps better to have the definition in .cpp file?

moved, it being (readily) inlined shouldn't be an issue.

[Raymond Toy, 2016-05-05 17:02:22]

On 2016/05/05 16:31:43, sof wrote:
>
https://codereview.chromium.org/1954663002/diff/1/third_party/WebKit/Source/m...
> File third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h (right):
> 
>
https://codereview.chromium.org/1954663002/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h:102: void
> setCurrentAsAudioThread()
> On 2016/05/05 15:58:49, Raymond Toy wrote:
> > I have a slight preference for setAudioThreadToCurrentThread.
> 
> renamed
> 
>
https://codereview.chromium.org/1954663002/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h:106:
> ASSERT(m_audioThread == currentThread());
> On 2016/05/05 15:58:49, Raymond Toy wrote:
> > Isn't it DCHECK() instead of ASSERT() now?
> 
> The current story is clouded; I prefer consistency per file & module, and
> ASSERT() is what's used.
> 
>
https://codereview.chromium.org/1954663002/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h:107: return;
> On 2016/05/05 15:58:49, Raymond Toy wrote:
> > I don't think it's possible for the assert to fail, but what should happen
if
> > the threads don't actually match?
> 
> isAudioThread() isn't prepared for that, so it should fail. An unlikely case
> best handled via a (non-release) assert, was how I reasoned.

Works for me.

[sof, 2016-05-05 17:39:52]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2016-05-05 17:39:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, rtoy@chromium.org, hongchan@chromium.org
Link to the patchset: https://codereview.chromium.org/1954663002/#ps40001
(title: "out-of-line setter")

[commit-bot: I haz the power, 2016-05-05 17:40:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1954663002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1954663002/40001

[commit-bot: I haz the power, 2016-05-05 18:25:36]

Description was changed from

==========
Initialize DeferredTaskHandler's audio thread ID once.

Avoid repeatedly recording the audio thread's ID, along with assertedly
verifying that this is done off the main thread.

R=
BUG=590108
==========

to

==========
Initialize DeferredTaskHandler's audio thread ID once.

Avoid repeatedly recording the audio thread's ID, along with assertedly
verifying that this is done off the main thread.

R=
BUG=590108
==========

[commit-bot: I haz the power, 2016-05-05 18:25:37]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-05 18:26:55]

Description was changed from

==========
Initialize DeferredTaskHandler's audio thread ID once.

Avoid repeatedly recording the audio thread's ID, along with assertedly
verifying that this is done off the main thread.

R=
BUG=590108
==========

to

==========
Initialize DeferredTaskHandler's audio thread ID once.

Avoid repeatedly recording the audio thread's ID, along with assertedly
verifying that this is done off the main thread.

R=
BUG=590108

Committed: https://crrev.com/cd465f7624dfd687c7506028a5c0553aa3e4cd1b
Cr-Commit-Position: refs/heads/master@{#391848}
==========

[commit-bot: I haz the power, 2016-05-05 18:26:56]

Patchset 3 (id:??) landed as
https://crrev.com/cd465f7624dfd687c7506028a5c0553aa3e4cd1b
Cr-Commit-Position: refs/heads/master@{#391848}