Move session into //blimp/client/app and update GN files.

Move session into //blimp/client/app and update GN files.

As part of making the Android Blimp APK compatible with the
//blimp/client/public APIs, this first CL moves the session code into
//blimp/client/app. This includes the GN-targets to ensure that they are
easily identifiable as belonging to the app directory.

This moves the APK-target itself, which means that the testing bots
needed to be updated with the new path to the target. In addition, the
lint suppressions needed to be updated with a new path.

The //blimp/client:blimp_unittests_java_deps (moved from //blimp), still
needs to be referred to directly by //blimp/BUILD.gn, because adding it
as a deps to //blimp/client:unit_tests on the Android platform does not
make the Java classes end up in the unit test APK. Other than this, the
rest of the targets have now been moved to //blimp/client and
//blimp/client/app.

Some targets have been renamed since they now reside in the app
directory.

Lastly, since the visibility parts of //blimp/client/core is cleaned up,
a bug was also filed for removing the visibility for internal core
targets for the engine browser tests.

BUG=651964, 653789
Committed: https://crrev.com/1ce09cbb95446995ca12230ccf86b3467d0259f1
Cr-Commit-Position: refs/heads/master@{#423825}

[nyquist, 2016-09-30 23:13:59]

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

[commit-bot: I haz the power, 2016-09-30 23:14:36]

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, 2016-09-30 23:23:07]

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

[commit-bot: I haz the power, 2016-09-30 23:23:07]

Dry run: Exceeded global retry quota

[nyquist, 2016-10-01 18:53:03]

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

[commit-bot: I haz the power, 2016-10-01 18:53:08]

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, 2016-10-01 19:08:38]

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

[commit-bot: I haz the power, 2016-10-01 19:08:39]

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

[nyquist, 2016-10-01 21:59:24]

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

[commit-bot: I haz the power, 2016-10-01 21:59:34]

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, 2016-10-01 22:31:01]

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

[commit-bot: I haz the power, 2016-10-01 22:31:04]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[nyquist, 2016-10-02 23:01:20]

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

[commit-bot: I haz the power, 2016-10-02 23:01:24]

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, 2016-10-02 23:45:00]

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

[commit-bot: I haz the power, 2016-10-02 23:45:00]

Dry run: This issue passed the CQ dry run.

[nyquist, 2016-10-03 04:11:33]

Description was changed from

==========
Move session into //blimp/client/app and update GN files.

BUG=651964
==========

to

==========
Move session into //blimp/client/app and update GN files.

As part of making the Android Blimp APK compatible with the
//blimp/client/public APIs, this first CL moves the session code into
//blimp/client/app. This includes the GN-targets to ensure that they are
easily identifiable as belonging to the app directory.

This moves the APK-target itself, which means that the testing bots
needed to be updated with the new path to the target. In addition, the
lint suppressions needed to be updated with a new path.

The //blimp/client:blimp_unittests_java_deps (moved from //blimp), still
needs to be referred to directly by //blimp/BUILD.gn, because adding it
as a deps to //blimp/client:unit_tests on the Android platform does not
make the Java classes end up in the unit test APK. Other than this, the
rest of the targets have now been moved to //blimp/client and
//blimp/client/app.

Some targets have been renamed since they now reside in the app
directory.

BUG=651964
==========

[nyquist, 2016-10-03 04:22:34]

nyquist@chromium.org changed reviewers:
+ agrieve@chromium.org, dpranke@chromium.org, dtrainor@chromium.org

[nyquist, 2016-10-03 04:22:35]

dtrainor: PTAL //blimp
dpranke: PTAL //build/android/lint and //testing/buildbot.


agrieve / dpranke: I would be happy to receive advise on an issue I had with GN
and a test-->group-->java_group-->android_library dependency chain. Basically,
the blimp_unittests_apk target ended up NOT having the Java-classes that
//blimp/client:blimp_unittests_java_deps deps on. Example error:
[FATAL:jni_android.cc(123)] Failed to find class org/chromium/net/GURLUtils
They all all registered here:
https://chromium.googlesource.com/chromium/src/+/516883acd2ca500fb7174d105124...

This works (patch set 4):
//blimp:blimp_unittests (test) --> //blimp/client:blimp_unittest_java_deps
(java_group) --> a bunch of android_library targets
This does not work, but I would like it to (patch set 3):
//blimp:blimp_unittests (test) --> //blimp/client:unit_tests (group) -->
//blimp/client:blimp_unittests_java_deps (java_group) --> a bunch of
android_library targets

[agrieve, 2016-10-03 18:42:45]

On 2016/10/03 04:22:35, nyquist wrote:
> dtrainor: PTAL //blimp
> dpranke: PTAL //build/android/lint and //testing/buildbot.
> 
> 
> agrieve / dpranke: I would be happy to receive advise on an issue I had with
GN
> and a test-->group-->java_group-->android_library dependency chain. Basically,
> the blimp_unittests_apk target ended up NOT having the Java-classes that
> //blimp/client:blimp_unittests_java_deps deps on. Example error:
> [FATAL:jni_android.cc(123)] Failed to find class org/chromium/net/GURLUtils
> They all all registered here:
>
https://chromium.googlesource.com/chromium/src/+/516883acd2ca500fb7174d105124...
> 
> This works (patch set 4):
> //blimp:blimp_unittests (test) --> //blimp/client:blimp_unittest_java_deps
> (java_group) --> a bunch of android_library targets
> This does not work, but I would like it to (patch set 3):
> //blimp:blimp_unittests (test) --> //blimp/client:unit_tests (group) -->
> //blimp/client:blimp_unittests_java_deps (java_group) --> a bunch of
> android_library targets

Java rules require that every link in the chain is of the java sort. You can't
use group(), only java_group().

[Dirk Pranke, 2016-10-03 19:56:16]

This basically lgtm. I think agrieve@ answered your question about groups. Let
me know if I missed something.

[nyquist, 2016-10-05 20:58:10]

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

[commit-bot: I haz the power, 2016-10-05 20:58:47]

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, 2016-10-05 21:59:41]

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

[commit-bot: I haz the power, 2016-10-05 21:59:41]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2016-10-07 06:36:49]

lgtm!

https://codereview.chromium.org/2387813002/diff/80001/blimp/client/core/conte...
File blimp/client/core/context/BUILD.gn (right):

https://codereview.chromium.org/2387813002/diff/80001/blimp/client/core/conte...
blimp/client/core/context/BUILD.gn:15: "//blimp/engine:browser_tests",
Is this blocked on porting over the final few unit tests to 0.6?  Should we tie
this to a bug for that?  Same with contents/ and other core targets that expose
browser_tests.  wdyt?

[nyquist, 2016-10-07 07:47:06]

Description was changed from

==========
Move session into //blimp/client/app and update GN files.

As part of making the Android Blimp APK compatible with the
//blimp/client/public APIs, this first CL moves the session code into
//blimp/client/app. This includes the GN-targets to ensure that they are
easily identifiable as belonging to the app directory.

This moves the APK-target itself, which means that the testing bots
needed to be updated with the new path to the target. In addition, the
lint suppressions needed to be updated with a new path.

The //blimp/client:blimp_unittests_java_deps (moved from //blimp), still
needs to be referred to directly by //blimp/BUILD.gn, because adding it
as a deps to //blimp/client:unit_tests on the Android platform does not
make the Java classes end up in the unit test APK. Other than this, the
rest of the targets have now been moved to //blimp/client and
//blimp/client/app.

Some targets have been renamed since they now reside in the app
directory.

BUG=651964
==========

to

==========
Move session into //blimp/client/app and update GN files.

As part of making the Android Blimp APK compatible with the
//blimp/client/public APIs, this first CL moves the session code into
//blimp/client/app. This includes the GN-targets to ensure that they are
easily identifiable as belonging to the app directory.

This moves the APK-target itself, which means that the testing bots
needed to be updated with the new path to the target. In addition, the
lint suppressions needed to be updated with a new path.

The //blimp/client:blimp_unittests_java_deps (moved from //blimp), still
needs to be referred to directly by //blimp/BUILD.gn, because adding it
as a deps to //blimp/client:unit_tests on the Android platform does not
make the Java classes end up in the unit test APK. Other than this, the
rest of the targets have now been moved to //blimp/client and
//blimp/client/app.

Some targets have been renamed since they now reside in the app
directory.

Lastly, since the visibility parts of //blimp/client/core is cleaned up,
a bug was also filed for removing the visibility for internal core
targets for the engine browser tests.

BUG=651964, 653789
==========

[nyquist, 2016-10-07 07:47:23]

https://codereview.chromium.org/2387813002/diff/80001/blimp/client/core/conte...
File blimp/client/core/context/BUILD.gn (right):

https://codereview.chromium.org/2387813002/diff/80001/blimp/client/core/conte...
blimp/client/core/context/BUILD.gn:15: "//blimp/engine:browser_tests",
On 2016/10/07 06:36:48, David Trainor wrote:
> Is this blocked on porting over the final few unit tests to 0.6?  Should we
tie
> this to a bug for that?  Same with contents/ and other core targets that
expose
> browser_tests.  wdyt?

Filed a new bug for this, and added a comment for all the browser_tests target
visibilities.

[nyquist, 2016-10-07 07:47:30]

The CQ bit was checked by nyquist@chromium.org

[nyquist, 2016-10-07 07:47:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2387813002/#ps120001
(title: "Rebased")

[commit-bot: I haz the power, 2016-10-07 07:47:47]

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, 2016-10-07 09:39:22]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-10-07 09:41:25]

Description was changed from

==========
Move session into //blimp/client/app and update GN files.

As part of making the Android Blimp APK compatible with the
//blimp/client/public APIs, this first CL moves the session code into
//blimp/client/app. This includes the GN-targets to ensure that they are
easily identifiable as belonging to the app directory.

This moves the APK-target itself, which means that the testing bots
needed to be updated with the new path to the target. In addition, the
lint suppressions needed to be updated with a new path.

The //blimp/client:blimp_unittests_java_deps (moved from //blimp), still
needs to be referred to directly by //blimp/BUILD.gn, because adding it
as a deps to //blimp/client:unit_tests on the Android platform does not
make the Java classes end up in the unit test APK. Other than this, the
rest of the targets have now been moved to //blimp/client and
//blimp/client/app.

Some targets have been renamed since they now reside in the app
directory.

Lastly, since the visibility parts of //blimp/client/core is cleaned up,
a bug was also filed for removing the visibility for internal core
targets for the engine browser tests.

BUG=651964, 653789
==========

to

==========
Move session into //blimp/client/app and update GN files.

As part of making the Android Blimp APK compatible with the
//blimp/client/public APIs, this first CL moves the session code into
//blimp/client/app. This includes the GN-targets to ensure that they are
easily identifiable as belonging to the app directory.

This moves the APK-target itself, which means that the testing bots
needed to be updated with the new path to the target. In addition, the
lint suppressions needed to be updated with a new path.

The //blimp/client:blimp_unittests_java_deps (moved from //blimp), still
needs to be referred to directly by //blimp/BUILD.gn, because adding it
as a deps to //blimp/client:unit_tests on the Android platform does not
make the Java classes end up in the unit test APK. Other than this, the
rest of the targets have now been moved to //blimp/client and
//blimp/client/app.

Some targets have been renamed since they now reside in the app
directory.

Lastly, since the visibility parts of //blimp/client/core is cleaned up,
a bug was also filed for removing the visibility for internal core
targets for the engine browser tests.

BUG=651964, 653789

Committed: https://crrev.com/1ce09cbb95446995ca12230ccf86b3467d0259f1
Cr-Commit-Position: refs/heads/master@{#423825}
==========

[commit-bot: I haz the power, 2016-10-07 09:41:27]

Patchset 7 (id:??) landed as
https://crrev.com/1ce09cbb95446995ca12230ccf86b3467d0259f1
Cr-Commit-Position: refs/heads/master@{#423825}