[Media Router] Implement an internal Media Routes observer.

[Media Router] Implement an internal Media Routes observer.

This observer has two jobs:

1. Keep track of whether there is a local route, so we know when to
pass JoinRoute requests to the MRPM.

2. Keep track of off-the-record route ids, so we know which routes
to terminate when the incognito profile is destroyed.

Feature #2 replaces the existing mechanism for tracking OTR route ids.

Note that this requires adding an Initialize() method to MediaRouterBase, as we
shouldn't create/destroy observers in the MR ctor/dtor.

TODO: Clean up some redundant code in the Mojo unittest.

BUG=599996
Committed: https://crrev.com/bdabddfbfaade62b4e734bb66af4e04dc14a5f6c
Cr-Commit-Position: refs/heads/master@{#390000}

[mark a. foltz, 2016-04-22 00:02:18]

mfoltz@chromium.org changed reviewers:
+ apacible@chromium.org, imcheng@chromium.org

[imcheng, 2016-04-25 18:59:54]

lgtm with a couple of suggestions. Sorry for the delay.

https://codereview.chromium.org/1911183002/diff/1/chrome/browser/media/router...
File chrome/browser/media/router/media_router_base.cc (right):

https://codereview.chromium.org/1911183002/diff/1/chrome/browser/media/router...
chrome/browser/media/router/media_router_base.cc:16:
MediaRouterBase::~MediaRouterBase() {}
suggestion: Add a CHECK(!internal_routes_observer_); here.

https://codereview.chromium.org/1911183002/diff/1/chrome/browser/media/router...
File chrome/browser/media/router/media_router_base.h (right):

https://codereview.chromium.org/1911183002/diff/1/chrome/browser/media/router...
chrome/browser/media/router/media_router_base.h:87: bool has_local_route;
nit: prefer moving member variables to private and exposing getters.

[apacible, 2016-04-25 19:28:57]

lgtm

[mark a. foltz, 2016-04-26 21:11:10]

https://codereview.chromium.org/1911183002/diff/1/chrome/browser/media/router...
File chrome/browser/media/router/media_router_base.cc (right):

https://codereview.chromium.org/1911183002/diff/1/chrome/browser/media/router...
chrome/browser/media/router/media_router_base.cc:16:
MediaRouterBase::~MediaRouterBase() {}
On 2016/04/25 at 18:59:54, imcheng wrote:
> suggestion: Add a CHECK(!internal_routes_observer_); here.

Done.

https://codereview.chromium.org/1911183002/diff/1/chrome/browser/media/router...
File chrome/browser/media/router/media_router_base.h (right):

https://codereview.chromium.org/1911183002/diff/1/chrome/browser/media/router...
chrome/browser/media/router/media_router_base.h:87: bool has_local_route;
On 2016/04/25 at 18:59:54, imcheng wrote:
> nit: prefer moving member variables to private and exposing getters.

This entire class is a private implementation detail of MediaRouterBase, I don't
think accessors add much value vs. code bloat here.

I did find the following advice and moved this to the implementation file.

http://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Move-in...

[mark a. foltz, 2016-04-26 21:11:55]

The CQ bit was checked by mfoltz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-26 21:12:29]

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

[commit-bot: I haz the power, 2016-04-26 21:25:14]

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

[commit-bot: I haz the power, 2016-04-26 21:25:15]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)

[mark a. foltz, 2016-04-26 23:35:34]

The CQ bit was checked by mfoltz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-26 23:35:53]

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

[commit-bot: I haz the power, 2016-04-27 00:54:16]

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

[commit-bot: I haz the power, 2016-04-27 00:54:17]

Dry run: This issue passed the CQ dry run.

[mark a. foltz, 2016-04-27 06:01:55]

The CQ bit was checked by mfoltz@chromium.org

[mark a. foltz, 2016-04-27 06:01:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
imcheng@chromium.org, apacible@chromium.org
Link to the patchset: https://codereview.chromium.org/1911183002/#ps40001
(title: "Fix bad merge & android build")

[commit-bot: I haz the power, 2016-04-27 06:02:06]

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

[commit-bot: I haz the power, 2016-04-27 06:10:56]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-27 06:12:28]

Description was changed from

==========
[Media Router] Implement an internal Media Routes observer.

This observer has two jobs:

1. Keep track of whether there is a local route, so we know when to
pass JoinRoute requests to the MRPM.

2. Keep track of off-the-record route ids, so we know which routes
to terminate when the incognito profile is destroyed.

Feature #2 replaces the existing mechanism for tracking OTR route ids.

Note that this requires adding an Initialize() method to MediaRouterBase, as we
shouldn't create/destroy observers in the MR ctor/dtor.

TODO: Clean up some redundant code in the Mojo unittest.

BUG=599996
==========

to

==========
[Media Router] Implement an internal Media Routes observer.

This observer has two jobs:

1. Keep track of whether there is a local route, so we know when to
pass JoinRoute requests to the MRPM.

2. Keep track of off-the-record route ids, so we know which routes
to terminate when the incognito profile is destroyed.

Feature #2 replaces the existing mechanism for tracking OTR route ids.

Note that this requires adding an Initialize() method to MediaRouterBase, as we
shouldn't create/destroy observers in the MR ctor/dtor.

TODO: Clean up some redundant code in the Mojo unittest.

BUG=599996

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

[commit-bot: I haz the power, 2016-04-27 06:12:30]

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