Cast: Don't create threads for each CastSessionDelegate

Cast: Don't create threads for each CastSessionDelegate

We shouldn't create threads for each instance of CastSessionDelegate.

Cast encoding threads are now managed by a singleton object of
CastThreads. Threads used by Cast are now started and stopped
accordingly when there are CastSessionDelegate objects.

Tested with browser_tests --gtest_filter=CastStreaming*

BUG=317006
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252810

[scherkus (not reviewing), 2014-02-21 16:25:48]

https://codereview.chromium.org/174403002/diff/2/chrome/renderer/media/cast_s...
File chrome/renderer/media/cast_session_delegate.cc (right):

https://codereview.chromium.org/174403002/diff/2/chrome/renderer/media/cast_s...
chrome/renderer/media/cast_session_delegate.cc:34:
g_cast_threads.Get().IncrementUsage();
do we ever create objects but not initialize them? this will create the threads
but potentially not use them until later

[Alpha Left Google, 2014-02-21 20:40:55]

On 2014/02/21 16:25:48, scherkus wrote:
>
https://codereview.chromium.org/174403002/diff/2/chrome/renderer/media/cast_s...
> File chrome/renderer/media/cast_session_delegate.cc (right):
> 
>
https://codereview.chromium.org/174403002/diff/2/chrome/renderer/media/cast_s...
> chrome/renderer/media/cast_session_delegate.cc:34:
> g_cast_threads.Get().IncrementUsage();
> do we ever create objects but not initialize them? this will create the
threads
> but potentially not use them until later

Right now we initialize CastSender during StartAudio/StartVideo. So yes we could
be starting the threads but not use them until later. This is going to change.
We're going (mikhal is doing the change right now) to create and initialize
CastSender in constructor of CastSessionDelegate. And so starting the threads in
the constructor is the right place.

[scherkus (not reviewing), 2014-02-21 22:01:03]

https://codereview.chromium.org/174403002/diff/2/chrome/renderer/media/cast_t...
File chrome/renderer/media/cast_threads.h (right):

https://codereview.chromium.org/174403002/diff/2/chrome/renderer/media/cast_t...
chrome/renderer/media/cast_threads.h:22: CastThreads();
these can be made private and have something like:

friend struct base::DefaultLazyInstanceTraits<CastThreads>;

https://codereview.chromium.org/174403002/diff/2/chrome/renderer/media/cast_t...
chrome/renderer/media/cast_threads.h:31: void IncrementUsage();
it might be overkill ... but you could declare a helper class that tracks usage

or perhaps it's not even worth cleaning up the threads after they've been
created seeing that this is a renderer process and will eventually get nuked

WDYT?

https://codereview.chromium.org/174403002/diff/2/chrome/renderer/media/cast_t...
chrome/renderer/media/cast_threads.h:41: };
DISALLOW etc...

[Alpha Left Google, 2014-02-22 01:04:33]

https://codereview.chromium.org/174403002/diff/2/chrome/renderer/media/cast_t...
File chrome/renderer/media/cast_threads.h (right):

https://codereview.chromium.org/174403002/diff/2/chrome/renderer/media/cast_t...
chrome/renderer/media/cast_threads.h:22: CastThreads();
On 2014/02/21 22:01:03, scherkus wrote:
> these can be made private and have something like:
> 
> friend struct base::DefaultLazyInstanceTraits<CastThreads>;

Done.

https://codereview.chromium.org/174403002/diff/2/chrome/renderer/media/cast_t...
chrome/renderer/media/cast_threads.h:31: void IncrementUsage();
On 2014/02/21 22:01:03, scherkus wrote:
> it might be overkill ... but you could declare a helper class that tracks
usage
> 
> or perhaps it's not even worth cleaning up the threads after they've been
> created seeing that this is a renderer process and will eventually get nuked
> 
> WDYT?

Okay I just create threads then this CastThreads is constructed.

https://codereview.chromium.org/174403002/diff/2/chrome/renderer/media/cast_t...
chrome/renderer/media/cast_threads.h:41: };
On 2014/02/21 22:01:03, scherkus wrote:
> DISALLOW etc...

Done.

[scherkus (not reviewing), 2014-02-22 01:08:27]

lgtm w/ suggestion

https://codereview.chromium.org/174403002/diff/100001/chrome/renderer/media/c...
File chrome/renderer/media/cast_threads.h (right):

https://codereview.chromium.org/174403002/diff/100001/chrome/renderer/media/c...
chrome/renderer/media/cast_threads.h:21: scoped_refptr<base::MessageLoopProxy>
GetAudioEncodeMessageLoopProxy();
should these be base::SingleThreadTaskRunners instead?

GetAudioEncodeTaskRunner() etc...

[scherkus (not reviewing), 2014-02-22 01:08:27]

lgtm w/ suggestion

[Alpha Left Google, 2014-02-22 01:13:10]

The CQ bit was checked by hclam@chromium.org

[Alpha Left Google, 2014-02-22 01:13:23]

On 2014/02/22 01:08:27, scherkus wrote:
> lgtm w/ suggestion

Done. Changed to SingleThreadTaskRunner.

[commit-bot: I haz the power, 2014-02-22 01:14:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/174403002/140002

[commit-bot: I haz the power, 2014-02-22 01:37:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/174403002/140002

[commit-bot: I haz the power, 2014-02-22 05:33:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-22 05:33:27]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Alpha Left Google, 2014-02-22 10:19:14]

The CQ bit was checked by hclam@chromium.org

[commit-bot: I haz the power, 2014-02-22 10:20:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/174403002/140002

[commit-bot: I haz the power, 2014-02-22 18:56:09]

Change committed as 252810