Media Engagement: implement shell of the media engagement service.

Media Engagement: implement shell of the media engagement service.

This is created the KeyedService, the WebContentsObserver and the
Feature flag. This is only boilerplate.

BUG=715051
Review-Url: https://codereview.chromium.org/2840813002
Cr-Commit-Position: refs/heads/master@{#470362}
Committed: https://chromium.googlesource.com/chromium/src/+/aba690d1d703028cb7f77b3ae7b8093c2cb8c087

[mlamouri (slow - plz ping), 2017-04-25 10:56:06]

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

[commit-bot: I haz the power, 2017-04-25 10:56:20]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mlamouri (slow - plz ping), 2017-04-25 12:00:52]

mlamouri@chromium.org changed reviewers:
+ dalecurtis@chromium.org, thakis@chromium.org

[mlamouri (slow - plz ping), 2017-04-25 12:00:53]

dalecurtis@ PTAL at media/ and chrome/browser/media/
thakis@ PTAL at chrome/browser/{profiles,ui}/

[commit-bot: I haz the power, 2017-04-25 12:43:58]

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

[commit-bot: I haz the power, 2017-04-25 12:43:59]

Dry run: This issue passed the CQ dry run.

[DaleCurtis, 2017-04-25 16:58:00]

Not much to review here, will keep an eye on future CLs, but shims lgtm.

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
File chrome/browser/media/media_engagement_service.cc (right):

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service.cc:16: explicit
ContentsObserver(content::WebContents* web_contents,
No explicit for 2 params.

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service.cc:29:
DISALLOW_COPY_AND_ASSIGN(ContentsObserver);
last.

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
File chrome/browser/media/media_engagement_service.h (right):

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service.h:18: class MediaEngagementService
: public KeyedService {
I thought we used namespaces in chrome/ now?

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service.h:30:
MediaEngagementService(Profile* profile);
explicit

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service.h:36: std::set<ContentsObserver*>
contents_observers_;
How many of these are expected? base::flat_set?

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service.h:37: };
DISALLOW_COPY_AND_ASSIGN().

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
File chrome/browser/media/media_engagement_service_factory.h (right):

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service_factory.h:31: };
DISALLOW...

https://codereview.chromium.org/2840813002/diff/1/media/base/media_switches.cc
File media/base/media_switches.cc (right):

https://codereview.chromium.org/2840813002/diff/1/media/base/media_switches.c...
media/base/media_switches.cc:204: // Enables Media Engagement Index recording in
order to by-pass autoplay
bypass

[mlamouri (slow - plz ping), 2017-04-25 17:14:33]

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

[mlamouri (slow - plz ping), 2017-04-25 17:14:41]

Comments applied.

thakis@, PTAL

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
File chrome/browser/media/media_engagement_service.cc (right):

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service.cc:16: explicit
ContentsObserver(content::WebContents* web_contents,
On 2017/04/25 at 16:57:59, DaleCurtis wrote:
> No explicit for 2 params.

Eh. As above :)

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service.cc:29:
DISALLOW_COPY_AND_ASSIGN(ContentsObserver);
On 2017/04/25 at 16:57:59, DaleCurtis wrote:
> last.

Done.

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
File chrome/browser/media/media_engagement_service.h (right):

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service.h:18: class MediaEngagementService
: public KeyedService {
On 2017/04/25 at 16:57:59, DaleCurtis wrote:
> I thought we used namespaces in chrome/ now?

I do not remember hearing about this and code around uses namespace randomly
(`chrome`, specific, none). Do you have a preference? I don't apart that `media`
might clash with //media.

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service.h:30:
MediaEngagementService(Profile* profile);
On 2017/04/25 at 16:57:59, DaleCurtis wrote:
> explicit

We need automatic checks for these. Too easy to add/remove parameters and forget
:(

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service.h:36: std::set<ContentsObserver*>
contents_observers_;
On 2017/04/25 at 16:57:59, DaleCurtis wrote:
> How many of these are expected? base::flat_set?

One per tab. I don't think flat_set is appropriate, from reading the doc.

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service.h:37: };
On 2017/04/25 at 16:57:59, DaleCurtis wrote:
> DISALLOW_COPY_AND_ASSIGN().

Done.

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
File chrome/browser/media/media_engagement_service_factory.h (right):

https://codereview.chromium.org/2840813002/diff/1/chrome/browser/media/media_...
chrome/browser/media/media_engagement_service_factory.h:31: };
On 2017/04/25 at 16:58:00, DaleCurtis wrote:
> DISALLOW...

Done.

https://codereview.chromium.org/2840813002/diff/1/media/base/media_switches.cc
File media/base/media_switches.cc (right):

https://codereview.chromium.org/2840813002/diff/1/media/base/media_switches.c...
media/base/media_switches.cc:204: // Enables Media Engagement Index recording in
order to by-pass autoplay
On 2017/04/25 at 16:58:00, DaleCurtis wrote:
> bypass

Done.

[commit-bot: I haz the power, 2017-04-25 17:15:01]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-25 19:26:31]

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

[commit-bot: I haz the power, 2017-04-25 19:26:32]

Dry run: This issue passed the CQ dry run.

[mlamouri (slow - plz ping), 2017-04-28 09:34:14]

thakis@, PTAL

[Nico, 2017-05-09 13:39:01]

lgtm

[mlamouri (slow - plz ping), 2017-05-09 14:14:26]

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

[commit-bot: I haz the power, 2017-05-09 14:14:45]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 16:12:41]

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

[commit-bot: I haz the power, 2017-05-09 16:12:42]

Dry run: This issue passed the CQ dry run.

[mlamouri (slow - plz ping), 2017-05-09 17:05:58]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2017-05-09 17:05:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org, thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2840813002/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2017-05-09 17:07:00]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 17:16:43]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494349558705500,
"parent_rev": "064434bb26e0cf8c916b11ecd988041bdb1f1c40", "commit_rev":
"aba690d1d703028cb7f77b3ae7b8093c2cb8c087"}

[commit-bot: I haz the power, 2017-05-09 17:16:59]

Description was changed from

==========
Media Engagement: implement shell of the media engagement service.

This is created the KeyedService, the WebContentsObserver and the
Feature flag. This is only boilerplate.

BUG=715051
==========

to

==========
Media Engagement: implement shell of the media engagement service.

This is created the KeyedService, the WebContentsObserver and the
Feature flag. This is only boilerplate.

BUG=715051

Review-Url: https://codereview.chromium.org/2840813002
Cr-Commit-Position: refs/heads/master@{#470362}
Committed:
https://chromium.googlesource.com/chromium/src/+/aba690d1d703028cb7f77b3ae7b8...
==========

[commit-bot: I haz the power, 2017-05-09 17:17:00]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/aba690d1d703028cb7f77b3ae7b8...