Move site_engagement.mojom to chrome/browser/engagement.

Move site_engagement.mojom to chrome/browser/engagement.

To allow use of the same SiteEngagementInfo structure by both the site
engagement UI and the important sites implementation, this CL moves the
Mojo definitions out of c/b/ui/webui/engagement.

BUG=703848
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2783433003
Cr-Commit-Position: refs/heads/master@{#460248}
Committed: https://chromium.googlesource.com/chromium/src/+/cd9d51b19b9de7d555e7f0d4c5b32deec102db82

[Wez, 2017-03-28 04:52:19]

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

[commit-bot: I haz the power, 2017-03-28 04:52:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Wez, 2017-03-28 04:53:31]

wez@chromium.org changed reviewers:
+ dominickn@chromium.org

[Wez, 2017-03-28 04:53:31]

PTAL

[commit-bot: I haz the power, 2017-03-28 04:56:38]

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

[commit-bot: I haz the power, 2017-03-28 04:56:39]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Wez, 2017-03-28 05:01:29]

wez@chromium.org changed reviewers:
+ dcheng@chromium.org, sky@chromium.org

[Wez, 2017-03-28 05:01:29]

Dear OWNERS:

sky: rubber-stamp for the c/b/ui/webui/... changes, plz.
dcheng: rubber-stamp LGTM for moving .mojom & updating OWNERS in the target
directory, plz.

[dcheng, 2017-03-28 05:03:00]

mojo lgtm, though the bots seem pretty mad =)

[Wez, 2017-03-28 05:12:34]

Yeah, the GN check stage is upset because the WebUI bits shouldn't be able
to "see" the newly-separated-out engagement service bits; everything is so
interdependent that I think I'll have to un-do the source_set() change,
which is a shame, otherwise I'll have cyclical dependencies.

On 28 March 2017 at 16:03, <dcheng@chromium.org> wrote:

> mojo lgtm, though the bots seem pretty mad =)
>
> https://codereview.chromium.org/2783433003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Wez, 2017-03-28 05:26:31]

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

[commit-bot: I haz the power, 2017-03-28 05:26:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Wez, 2017-03-28 05:40:39]

Description was changed from

==========
Move site_engagement.mojom to chrome/browser/engagement.

To allow use of the same SiteEngagementInfo structure by both the site
engagement UI and the important sites implementation, this CL moves the
Mojo definitions out of c/b/ui/webui/engagement.

The site engagement service' implementation files are also moved to a
source-set in the new chrome/browser/engagement/BUILD.gn.

BUG=703848
==========

to

==========
Move site_engagement.mojom to chrome/browser/engagement.

To allow use of the same SiteEngagementInfo structure by both the site
engagement UI and the important sites implementation, this CL moves the
Mojo definitions out of c/b/ui/webui/engagement.

BUG=703848
==========

[commit-bot: I haz the power, 2017-03-28 06:22:30]

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

[commit-bot: I haz the power, 2017-03-28 06:22:31]

Dry run: This issue passed the CQ dry run.

[Wez, 2017-03-28 08:57:23]

Description was changed from

==========
Move site_engagement.mojom to chrome/browser/engagement.

To allow use of the same SiteEngagementInfo structure by both the site
engagement UI and the important sites implementation, this CL moves the
Mojo definitions out of c/b/ui/webui/engagement.

BUG=703848
==========

to

==========
Move site_engagement.mojom to chrome/browser/engagement.

To allow use of the same SiteEngagementInfo structure by both the site
engagement UI and the important sites implementation, this CL moves the
Mojo definitions out of c/b/ui/webui/engagement.

BUG=703848
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Wez, 2017-03-28 08:57:25]

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

[commit-bot: I haz the power, 2017-03-28 08:57:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Wez, 2017-03-28 09:00:22]

Worryingly, the previous patch-set passed the dry-run, but was actually broken,
due to a couple of instances of the old path that I'd missed.

Looks like we need to also add a browser-test to sanity-check that the page
basically works?

[dominickn, 2017-03-28 09:08:19]

On 2017/03/28 09:00:22, Wez wrote:
> Worryingly, the previous patch-set passed the dry-run, but was actually
broken,
> due to a couple of instances of the old path that I'd missed.
> 
> Looks like we need to also add a browser-test to sanity-check that the page
> basically works?

Adding a browser test for chrome://site-engagement has been on the todo list for
quite a while (one of the giant refactor-everything-mojo-with-json-files patches
broke it a while ago). Unfortunately I haven't had cycles and neither has any
one else until very recently. :(

Which makes me very happy that you're helping out here!

[dominickn, 2017-03-28 09:09:37]

lgtm, thanks!

https://codereview.chromium.org/2783433003/diff/40001/chrome/browser/engageme...
File chrome/browser/engagement/BUILD.gn (right):

https://codereview.chromium.org/2783433003/diff/40001/chrome/browser/engageme...
chrome/browser/engagement/BUILD.gn:1: # Copyright 2015 The Chromium Authors. All
rights reserved.
Nit: 2017

https://codereview.chromium.org/2783433003/diff/40001/chrome/browser/engageme...
File chrome/browser/engagement/site_engagement.mojom (right):

https://codereview.chromium.org/2783433003/diff/40001/chrome/browser/engageme...
chrome/browser/engagement/site_engagement.mojom:1: // Copyright 2015 The
Chromium Authors. All rights reserved.
Nit: 2017

[Wez, 2017-03-28 09:15:21]

https://codereview.chromium.org/2783433003/diff/40001/chrome/browser/engageme...
File chrome/browser/engagement/BUILD.gn (right):

https://codereview.chromium.org/2783433003/diff/40001/chrome/browser/engageme...
chrome/browser/engagement/BUILD.gn:1: # Copyright 2015 The Chromium Authors. All
rights reserved.
On 2017/03/28 09:09:37, dominickn wrote:
> Nit: 2017

This is really just a file move, so 2015 seems reasonable.

https://codereview.chromium.org/2783433003/diff/40001/chrome/browser/engageme...
File chrome/browser/engagement/site_engagement.mojom (right):

https://codereview.chromium.org/2783433003/diff/40001/chrome/browser/engageme...
chrome/browser/engagement/site_engagement.mojom:1: // Copyright 2015 The
Chromium Authors. All rights reserved.
On 2017/03/28 09:09:37, dominickn wrote:
> Nit: 2017

Similarly, this is just a file move, so the date shouldn't change.

[Wez, 2017-03-28 09:15:35]

The CQ bit was unchecked by wez@chromium.org

[Wez, 2017-03-28 09:15:35]

The CQ bit was checked by wez@chromium.org

[Wez, 2017-03-28 09:15:36]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2783433003/#ps40001
(title: "Fix missed references to the old path")

[commit-bot: I haz the power, 2017-03-28 09:15:39]

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

[Wez, 2017-03-28 09:15:41]

The CQ bit was unchecked by wez@chromium.org

[Wez, 2017-03-28 09:15:57]

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

[commit-bot: I haz the power, 2017-03-28 09:16:13]

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-03-28 09:51:46]

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

[commit-bot: I haz the power, 2017-03-28 09:51:47]

Dry run: This issue passed the CQ dry run.

[sky, 2017-03-28 17:17:56]

LGTM

[Wez, 2017-03-28 23:49:17]

The CQ bit was checked by wez@chromium.org

[commit-bot: I haz the power, 2017-03-28 23:49:58]

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

[Wez, 2017-03-28 23:54:12]

The CQ bit was unchecked by wez@chromium.org

[Wez, 2017-03-28 23:54:30]

The CQ bit was checked by wez@chromium.org

[commit-bot: I haz the power, 2017-03-28 23:55:28]

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-03-29 00:23:04]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490745270949030,
"parent_rev": "63b01cc47b07f79243a4a90905c7182bcc6672e6", "commit_rev":
"cd9d51b19b9de7d555e7f0d4c5b32deec102db82"}

[commit-bot: I haz the power, 2017-03-29 00:24:29]

Description was changed from

==========
Move site_engagement.mojom to chrome/browser/engagement.

To allow use of the same SiteEngagementInfo structure by both the site
engagement UI and the important sites implementation, this CL moves the
Mojo definitions out of c/b/ui/webui/engagement.

BUG=703848
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Move site_engagement.mojom to chrome/browser/engagement.

To allow use of the same SiteEngagementInfo structure by both the site
engagement UI and the important sites implementation, this CL moves the
Mojo definitions out of c/b/ui/webui/engagement.

BUG=703848
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[commit-bot: I haz the power, 2017-03-29 00:24:31]

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