Add render-side manager for MediaSession

Add render-side manager for MediaSession

RendererMediaSessionManager is responsible for storing all
user-created media sessions and keeping track for id allocation for
those.

BUG=497735
Committed: https://crrev.com/859f4f9553797ad1be0718eae2475aa0a614f332
Cr-Commit-Position: refs/heads/master@{#361973}

[davve, 2015-11-12 17:23:57]

The CQ bit was checked by davve@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-11-12 17:24:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1436243002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1436243002/1

[commit-bot: I haz the power, 2015-11-12 17:57:17]

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

[commit-bot: I haz the power, 2015-11-12 17:57:18]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[davve, 2015-11-13 13:32:26]

The CQ bit was checked by davve@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-11-13 13:32:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1436243002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1436243002/60001

[davve, 2015-11-13 14:22:08]

The CQ bit was checked by davve@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-11-13 14:22:15]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1436243002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1436243002/80001

[davve, 2015-11-13 14:24:40]

Description was changed from

==========
Create RendererMediaSessionManager stub

BUG=497735
==========

to

==========
Add RendererMediaSessionManager for MediaSessions

RendererMediaSessionManager is responsible for storing all
user-created media sessions and keeping track for id allocation for
those.

BUG=497735
==========

[davve, 2015-11-13 14:30:25]

Description was changed from

==========
Add RendererMediaSessionManager for MediaSessions

RendererMediaSessionManager is responsible for storing all
user-created media sessions and keeping track for id allocation for
those.

BUG=497735
==========

to

==========
Add render-side manager for MediaSession

RendererMediaSessionManager is responsible for storing all
user-created media sessions and keeping track for id allocation for
those.

BUG=497735
==========

[davve, 2015-11-13 14:30:49]

davve@opera.com changed reviewers:
+ avayvod@chromium.org, mlamouri@chromium.org, philipj@opera.com

[commit-bot: I haz the power, 2015-11-13 15:25:41]

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

[commit-bot: I haz the power, 2015-11-13 15:25:42]

Dry run: This issue passed the CQ dry run.

[philipj_slow, 2015-11-16 14:06:22]

Non-owner LGTM

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/media/...
File content/renderer/media/android/renderer_media_session_manager.cc (right):

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/media/...
content/renderer/media/android/renderer_media_session_manager.cc:12: const int
kDefaultMediaSession = 0;  // ID of the default media session
Perhaps append ID to this name?

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/media/...
content/renderer/media/android/renderer_media_session_manager.cc:28: return
next_session_id_++;
Maybe CHECK that this doesn't overflow so that it just crashes if it ever does?
Would like to hear what the other reviewers say about that.

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/media/...
File content/renderer/media/android/webmediasession_android_unittest.cc (right):

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/media/...
content/renderer/media/android/webmediasession_android_unittest.cc:31: new
RendererMediaSessionManager(NULL));
nullptr?

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/render...
File content/renderer/render_frame_impl.h (right):

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/render...
content/renderer/render_frame_impl.h:1022: // RenderFrameObserver.
Plural?

[davve, 2015-11-16 14:34:27]

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/media/...
File content/renderer/media/android/renderer_media_session_manager.cc (right):

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/media/...
content/renderer/media/android/renderer_media_session_manager.cc:12: const int
kDefaultMediaSession = 0;  // ID of the default media session
On 2015/11/16 14:06:21, philipj wrote:
> Perhaps append ID to this name?

Done.

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/media/...
content/renderer/media/android/renderer_media_session_manager.cc:28: return
next_session_id_++;
On 2015/11/16 14:06:21, philipj wrote:
> Maybe CHECK that this doesn't overflow so that it just crashes if it ever
does?
> Would like to hear what the other reviewers say about that.

Adding a check sounds good, but I'm also curious what other reviewers think.

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/media/...
File content/renderer/media/android/webmediasession_android_unittest.cc (right):

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/media/...
content/renderer/media/android/webmediasession_android_unittest.cc:31: new
RendererMediaSessionManager(NULL));
On 2015/11/16 14:06:21, philipj wrote:
> nullptr?

Done.

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/render...
File content/renderer/render_frame_impl.h (right):

https://codereview.chromium.org/1436243002/diff/80001/content/renderer/render...
content/renderer/render_frame_impl.h:1022: // RenderFrameObserver.
On 2015/11/16 14:06:21, philipj wrote:
> Plural?

Done.

[davve, 2015-11-18 08:21:11]

The CQ bit was checked by davve@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-11-18 08:21:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1436243002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1436243002/120001

[commit-bot: I haz the power, 2015-11-18 09:24:59]

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

[commit-bot: I haz the power, 2015-11-18 09:25:00]

Dry run: This issue passed the CQ dry run.

[davve, 2015-11-23 13:16:02]

ping

[mlamouri (slow - plz ping), 2015-11-24 14:25:07]

Sorry for the delay, I was travelling the last couple of weeks.

lgtm with comments addressed.

https://codereview.chromium.org/1436243002/diff/120001/content/renderer/media...
File content/renderer/media/android/webmediasession_android_unittest.cc (right):

https://codereview.chromium.org/1436243002/diff/120001/content/renderer/media...
content/renderer/media/android/webmediasession_android_unittest.cc:29:
TEST_F(WebMediaSessionTest, TestRegistration) {
Can you add more tests. For example, create two sessions, unregister one and
check that the manager still has one session and it's the correct one.

Feel free to add a lot of unit tests. They are cheap and helpful! :)

[davve, 2015-11-24 16:49:50]

The CQ bit was checked by davve@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-11-24 16:51:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1436243002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1436243002/140001

[davve, 2015-11-24 16:52:06]

davve@opera.com changed reviewers:
+ davidben@chromium.org, qinmin@chromium.org

[davve, 2015-11-24 16:52:07]

https://codereview.chromium.org/1436243002/diff/120001/content/renderer/media...
File content/renderer/media/android/webmediasession_android_unittest.cc (right):

https://codereview.chromium.org/1436243002/diff/120001/content/renderer/media...
content/renderer/media/android/webmediasession_android_unittest.cc:29:
TEST_F(WebMediaSessionTest, TestRegistration) {
On 2015/11/24 14:25:07, Mounir Lamouri wrote:
> Can you add more tests. For example, create two sessions, unregister one and
> check that the manager still has one session and it's the correct one.
> 
> Feel free to add a lot of unit tests. They are cheap and helpful! :)

Not that much to tests yet, but note taken. Two more added I will try to add
more in the future.

[davidben, 2015-11-24 17:26:00]

(Which files did you want me to review? The whole thing or do you have OWNERS
coverage over some of it already?)

[commit-bot: I haz the power, 2015-11-24 18:35:42]

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

[commit-bot: I haz the power, 2015-11-24 18:35:43]

Dry run: This issue passed the CQ dry run.

[davve, 2015-11-24 19:26:53]

On 2015/11/24 17:26:00, davidben (slow) wrote:
> (Which files did you want me to review? The whole thing or do you have OWNERS
> coverage over some of it already?)

Sorry for not spelling it out.

git cl owners suggested qinmin for content/renderer/media/android/ and you for
the rest.

[davve, 2015-11-26 08:10:55]

davve@opera.com changed reviewers:
+ jochen@chromium.org

[davve, 2015-11-26 08:10:55]

Jochen, do you think you can rubberstamp this when you have a moment?

[jochen (gone - plz use gerrit), 2015-11-26 13:20:44]

lgtm

[davve, 2015-11-26 13:21:49]

The CQ bit was checked by davve@opera.com

[davve, 2015-11-26 13:21:50]

The patchset sent to the CQ was uploaded after l-g-t-m from philipj@opera.com,
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/1436243002/#ps140001
(title: "Moar tests")

[commit-bot: I haz the power, 2015-11-26 13:21:54]

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

[commit-bot: I haz the power, 2015-11-26 16:37:16]

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

[commit-bot: I haz the power, 2015-11-26 16:37:17]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

[davve, 2015-11-26 23:36:36]

The CQ bit was checked by davve@opera.com

[commit-bot: I haz the power, 2015-11-26 23:37:04]

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

[commit-bot: I haz the power, 2015-11-27 02:00:29]

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

[commit-bot: I haz the power, 2015-11-27 02:00:30]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

[davve, 2015-11-27 10:10:44]

The CQ bit was checked by davve@opera.com

[commit-bot: I haz the power, 2015-11-27 10:11:27]

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

[commit-bot: I haz the power, 2015-11-27 10:16:25]

Description was changed from

==========
Add render-side manager for MediaSession

RendererMediaSessionManager is responsible for storing all
user-created media sessions and keeping track for id allocation for
those.

BUG=497735
==========

to

==========
Add render-side manager for MediaSession

RendererMediaSessionManager is responsible for storing all
user-created media sessions and keeping track for id allocation for
those.

BUG=497735
==========

[commit-bot: I haz the power, 2015-11-27 10:16:26]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-11-27 10:17:14]

Description was changed from

==========
Add render-side manager for MediaSession

RendererMediaSessionManager is responsible for storing all
user-created media sessions and keeping track for id allocation for
those.

BUG=497735
==========

to

==========
Add render-side manager for MediaSession

RendererMediaSessionManager is responsible for storing all
user-created media sessions and keeping track for id allocation for
those.

BUG=497735

Committed: https://crrev.com/859f4f9553797ad1be0718eae2475aa0a614f332
Cr-Commit-Position: refs/heads/master@{#361973}
==========

[commit-bot: I haz the power, 2015-11-27 10:17:15]

Patchset 8 (id:??) landed as
https://crrev.com/859f4f9553797ad1be0718eae2475aa0a614f332
Cr-Commit-Position: refs/heads/master@{#361973}