Make mojom targets depend on chromium mojo environment

Make mojom targets depend on chromium mojo environment

BUG=501385
Committed: https://crrev.com/40ba3ca8ecf6588a908263bce867d1aa72c8c24e
Cr-Commit-Position: refs/heads/master@{#353955}

[Ken Rockot(use gerrit already), 2015-10-13 16:29:32]

Patchset #1 (id:1) has been deleted

[Ken Rockot(use gerrit already), 2015-10-13 17:09:12]

Patchset #1 (id:20001) has been deleted

[Ken Rockot(use gerrit already), 2015-10-13 17:45:18]

Patchset #1 (id:40001) has been deleted

[Ken Rockot(use gerrit already), 2015-10-13 18:07:03]

rockot@chromium.org changed reviewers:
+ sky@chromium.org, thakis@chromium.org

[Ken Rockot(use gerrit already), 2015-10-13 18:07:03]

sky@ please review from a mojo/mandoline perspective. Since we've forked and
only have one environment impl between chromium and mandoline, it seems
reasonable to make this implicit dependency the default behavior. The
alternative is that every target dependent on a mojom must also depend on
//mojo/environment:chromium, which means often underspecified deps. WDYT?

thakis@ please review for general GN

[Ken Rockot(use gerrit already), 2015-10-13 18:08:32]

(CL description updated to match what it actually does now...)

[Nico, 2015-10-13 18:12:28]

gn wise, this lgtm

What does "forked" mean here? Is mojo in chromium now independent of upstream
mojo? Can I read more about this somewhere?

[Ken Rockot(use gerrit already), 2015-10-13 18:17:16]

On 2015/10/13 18:12:28, Nico wrote:
> gn wise, this lgtm
> 
> What does "forked" mean here? Is mojo in chromium now independent of upstream
> mojo? Can I read more about this somewhere?

That's what it means, but I'm not sure there's been much documented discussion.
We're no longer rolling //third_party/mojo and will migrate back to the
implementation in //mojo once it's ready.

[Nico, 2015-10-13 18:19:17]

What's upstream mojo there for then, if not chromium?

[sky, 2015-10-13 19:58:49]

This seems reasonable, my only quibble is the naming.

https://codereview.chromium.org/1400123002/diff/60001/third_party/mojo/src/mo...
File third_party/mojo/src/mojo/public/tools/bindings/mojom.gni (right):

https://codereview.chromium.org/1400123002/diff/60001/third_party/mojo/src/mo...
third_party/mojo/src/mojo/public/tools/bindings/mojom.gni:41: #  
without_environment (optional)
I'm not a fan of negative booleans (NDEBUG is the worst in my opinion). Also,
search for "Episode 387: Improving Readability With Positive Booleans" on
internal pages too (in case you missed the episode).

Can you invert it?

[Ken Rockot(use gerrit already), 2015-10-13 20:06:12]

On 2015/10/13 19:58:49, sky wrote:
> This seems reasonable, my only quibble is the naming.
> 
>
https://codereview.chromium.org/1400123002/diff/60001/third_party/mojo/src/mo...
> File third_party/mojo/src/mojo/public/tools/bindings/mojom.gni (right):
> 
>
https://codereview.chromium.org/1400123002/diff/60001/third_party/mojo/src/mo...
> third_party/mojo/src/mojo/public/tools/bindings/mojom.gni:41: #  
> without_environment (optional)
> I'm not a fan of negative booleans (NDEBUG is the worst in my opinion). Also,
> search for "Episode 387: Improving Readability With Positive Booleans" on
> internal pages too (in case you missed the episode).
> 
> Can you invert it?

On 2015/10/13 19:58:49, sky wrote:
> This seems reasonable, my only quibble is the naming.
> 
>
https://codereview.chromium.org/1400123002/diff/60001/third_party/mojo/src/mo...
> File third_party/mojo/src/mojo/public/tools/bindings/mojom.gni (right):
> 
>
https://codereview.chromium.org/1400123002/diff/60001/third_party/mojo/src/mo...
> third_party/mojo/src/mojo/public/tools/bindings/mojom.gni:41: #  
> without_environment (optional)
> I'm not a fan of negative booleans (NDEBUG is the worst in my opinion). Also,
> search for "Episode 387: Improving Readability With Positive Booleans" on
> internal pages too (in case you missed the episode).
> 
> Can you invert it?

I agree. I thought about (and shamelessly dismissed) changing it after
uploading. You have successfully countered my laziness! Fixed.

[Ken Rockot(use gerrit already), 2015-10-13 22:13:45]

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

[commit-bot: I haz the power, 2015-10-13 22:18:38]

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

[commit-bot: I haz the power, 2015-10-13 22:43:42]

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

[commit-bot: I haz the power, 2015-10-13 22:43:43]

Dry run: This issue passed the CQ dry run.

[sky, 2015-10-14 02:33:31]

LGTM

[Ken Rockot(use gerrit already), 2015-10-14 03:48:09]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2015-10-14 03:48:10]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/1400123002/#ps80001
(title: "don't not invert the boolean")

[commit-bot: I haz the power, 2015-10-14 03:48:59]

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

[commit-bot: I haz the power, 2015-10-14 03:53:37]

Committed patchset #2 (id:80001)

[commit-bot: I haz the power, 2015-10-14 03:54:42]

Patchset 2 (id:??) landed as
https://crrev.com/40ba3ca8ecf6588a908263bce867d1aa72c8c24e
Cr-Commit-Position: refs/heads/master@{#353955}