Merge device/vibration/android/BUILD.gn to vibration/BUILD.gn.

Merge device/vibration/android/BUILD.gn to vibration/BUILD.gn.

Removes need for allowing circular includes.

Committed: https://crrev.com/ae3632b7969f2db58748525b4d7e2ab474fd5859
Cr-Commit-Position: refs/heads/master@{#329865}

[scheib, 2015-05-13 22:48:02]

Patchset #1 (id:1) has been deleted

[scheib, 2015-05-13 23:20:07]

scheib@chromium.org changed reviewers:
+ timvolodine@chromium.org

[timvolodine, 2015-05-14 12:16:36]

hmm a lot of "if (is_android)", but I guess it's ok if this avoids the need for
circular dependencies. So the reason for circular dependencies in the first
place was because of android including the mojo generated header right? in which
case other apis may hit the same issue in the future if they get converted to
use mojo..

https://codereview.chromium.org/1123283007/diff/20001/device/vibration/BUILD.gn
File device/vibration/BUILD.gn (right):

https://codereview.chromium.org/1123283007/diff/20001/device/vibration/BUILD....
device/vibration/BUILD.gn:17: "android/vibration_jni_registrar.cc",
looks like this is only needed for android?

https://codereview.chromium.org/1123283007/diff/20001/device/vibration/BUILD....
device/vibration/BUILD.gn:39: sources += [ "vibration_manager_impl_default.cc" ]
nit: no need for else if you use sources -= [
"vibration_manager_impl_default.cc" ], but I don't feel strongly about it ;)

[scheib, 2015-05-14 16:42:52]

On 2015/05/14 12:16:36, timvolodine wrote:
> hmm a lot of "if (is_android)", but I guess it's ok if this avoids the need
for
> circular dependencies. So the reason for circular dependencies in the first
> place was because of android including the mojo generated header right? in
which
> case other apis may hit the same issue in the future if they get converted to
> use mojo..

Circular deps came from the JNI being built in a separate GN file, but requiring
access to the main target headers. When the include rules were enforced brettw
did a quick fix by adding circular includes escape hatch here. In recent
addition of JNI to device/bluetooth, where I had patterned JNI setup from
device/vibration, we came to the conclusion we shouldn't have multiple BUILD.gn
with circular includes and instead have just one.
https://codereview.chromium.org/1119113002/

[scheib, 2015-05-14 16:43:01]

https://codereview.chromium.org/1123283007/diff/20001/device/vibration/BUILD.gn
File device/vibration/BUILD.gn (right):

https://codereview.chromium.org/1123283007/diff/20001/device/vibration/BUILD....
device/vibration/BUILD.gn:17: "android/vibration_jni_registrar.cc",
On 2015/05/14 12:16:36, timvolodine wrote:
> looks like this is only needed for android?

As in GYP, platform files are implicitly filtered:
See set_sources_assignment_filter
https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/referen...
https://code.google.com/p/chromium/codesearch#chromium/src/build/config/BUILD...

https://codereview.chromium.org/1123283007/diff/20001/device/vibration/BUILD....
device/vibration/BUILD.gn:39: sources += [ "vibration_manager_impl_default.cc" ]
On 2015/05/14 12:16:36, timvolodine wrote:
> nit: no need for else if you use sources -= [
> "vibration_manager_impl_default.cc" ], but I don't feel strongly about it ;)

GN style guide: List sources only once:
https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/style_g...

[scheib, 2015-05-14 16:48:16]

https://codereview.chromium.org/1123283007/diff/20001/device/vibration/BUILD.gn
File device/vibration/BUILD.gn (right):

https://codereview.chromium.org/1123283007/diff/20001/device/vibration/BUILD....
device/vibration/BUILD.gn:17: "android/vibration_jni_registrar.cc",
On 2015/05/14 16:43:00, scheib wrote:
> On 2015/05/14 12:16:36, timvolodine wrote:
> > looks like this is only needed for android?
> 
> As in GYP, platform files are implicitly filtered:
> See set_sources_assignment_filter
>
https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/referen...
>
https://code.google.com/p/chromium/codesearch#chromium/src/build/config/BUILD...

Actually, since the if (is_android) block below is already modifying sources,
I've moved the android sources there too.

[timvolodine, 2015-05-14 16:56:40]

ok thanks! lgtm

[scheib, 2015-05-14 17:06:35]

The CQ bit was checked by scheib@chromium.org

[commit-bot: I haz the power, 2015-05-14 17:07:29]

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

[commit-bot: I haz the power, 2015-05-14 17:20:00]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-05-14 17:20:47]

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