Java versions of BlimpContents[,Observer] and BlimpNavigationController

Add BlimpContents[,Observer] and BlimpNavigationController

This CL adds the public API for //blimp/client/core and adds a simple
implementation with a test for the core functionality for Java.

The setup of BlimpContents and related classes is intentionally
similar to how the Java version of the //content API is organized.

Users of this API are supposed to depend on the code in
//blimp/client/core/public, and not the implementation which lives
in //blimp/client/core directly.

An extra class compared to the C++ version is added for creating
BlimpContents, the BlimpContentsFactory. This sets up all the pointers
and references in the same ways as a BlimpContents created in native
code, but adds the requirement for Java code to invoke the destroy()
method of a BlimpContents.

BUG=611103
Committed: https://crrev.com/986f860c8fc6361d727a9ccd3c8cd44727b6f5ca
Cr-Commit-Position: refs/heads/master@{#401170}

[nyquist, 2016-06-10 01:37:16]

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

[commit-bot: I haz the power, 2016-06-10 01:38:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2059443002/20001

[commit-bot: I haz the power, 2016-06-10 01:43:44]

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

[commit-bot: I haz the power, 2016-06-10 01:43:45]

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

[nyquist, 2016-06-10 23:42:52]

Description was changed from

==========
Java versions of BlimpContents[,Observer] and BlimpNavigationController

This CL adds the Java-version of the public API for //blimp/client/core
and adds a simple implementation for the core functionality.

BUG=611103
==========

to

==========
Add BlimpContents[,Observer] and BlimpNavigationController

This CL adds the public API for //blimp/client/core and adds a simple
implementation with a test for the core functionality for Java.

The setup of BlimpContents and related classes is intentionally
similar to how the Java version of the //content API is organized.

Users of this API are supposed to depend on the code in
//blimp/client/core/public, and not the implementation which lives
in //blimp/client/core directly.

An extra class compared to the C++ version is added for creating
BlimpContents, the BlimpContentsFactory. This sets up all the pointers
and references in the same ways as a BlimpContents created in native
code, but adds the requirement for Java code to invoke the destroy()
method of a BlimpContents.

BUG=611103
==========

[nyquist, 2016-06-10 23:43:09]

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

[commit-bot: I haz the power, 2016-06-10 23:44:02]

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

[commit-bot: I haz the power, 2016-06-11 01:39:39]

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

[commit-bot: I haz the power, 2016-06-11 01:39:39]

Dry run: This issue passed the CQ dry run.

[nyquist, 2016-06-11 02:08:30]

nyquist@chromium.org changed reviewers:
+ dtrainor@chromium.org

[nyquist, 2016-06-11 02:08:31]

dtrainor: PTAL

[David Trainor- moved to gerrit, 2016-06-20 17:24:22]

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/app/androi...
File
blimp/client/app/android/javatests/src/org/chromium/blimp/BlimpNativeInstrumentationTestCase.java
(right):

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/app/androi...
blimp/client/app/android/javatests/src/org/chromium/blimp/BlimpNativeInstrumentationTestCase.java:21:
public void setUp() throws ProcessInitException {
is this @Override?

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/app/androi...
blimp/client/app/android/javatests/src/org/chromium/blimp/BlimpNativeInstrumentationTestCase.java:31:
protected final void waitUntilNativeIsReady() throws InterruptedException {
javadoc

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/andro...
File blimp/client/core/android/blimp_contents_factory.h (right):

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/andro...
blimp/client/core/android/blimp_contents_factory.h:1: // Copyright 2015 The
Chromium Authors. All rights reserved.
2016

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/andro...
File blimp/client/core/android/blimp_contents_impl_android.h (right):

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/andro...
blimp/client/core/android/blimp_contents_impl_android.h:32:
BlimpNavigationControllerImplAndroid
Should we just query this through blimp_contents_impl_?

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/andro...
File
blimp/client/core/android/java/src/org/chromium/blimp/core/BlimpContentsImpl.java
(right):

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/andro...
blimp/client/core/android/java/src/org/chromium/blimp/core/BlimpContentsImpl.java:78:
nativeDestroy(mNativeBlimpContentsImplAndroid);
assert mNativeBlimpContentsImplAndroid != 0?

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/blimp...
File blimp/client/core/blimp_contents_impl.cc (right):

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/blimp...
blimp/client/core/blimp_contents_impl.cc:33: delete blimp_contents_impl_android;
Will this crash?  Doesn't the underlying map actually own the user data?

[nyquist, 2016-06-21 20:27:20]

dtrainor: PTAL

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/app/androi...
File
blimp/client/app/android/javatests/src/org/chromium/blimp/BlimpNativeInstrumentationTestCase.java
(right):

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/app/androi...
blimp/client/app/android/javatests/src/org/chromium/blimp/BlimpNativeInstrumentationTestCase.java:21:
public void setUp() throws ProcessInitException {
On 2016/06/20 17:24:22, David Trainor wrote:
> is this @Override?

Why, yes it is, good sir.

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/app/androi...
blimp/client/app/android/javatests/src/org/chromium/blimp/BlimpNativeInstrumentationTestCase.java:31:
protected final void waitUntilNativeIsReady() throws InterruptedException {
On 2016/06/20 17:24:22, David Trainor wrote:
> javadoc

Done.

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/andro...
File blimp/client/core/android/blimp_contents_factory.h (right):

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/andro...
blimp/client/core/android/blimp_contents_factory.h:1: // Copyright 2015 The
Chromium Authors. All rights reserved.
On 2016/06/20 17:24:22, David Trainor wrote:
> 2016

I totally was confused by the year. Totally did not copy this from somewhere
else....

Done.

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/andro...
File blimp/client/core/android/blimp_contents_impl_android.h (right):

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/andro...
blimp/client/core/android/blimp_contents_impl_android.h:32:
BlimpNavigationControllerImplAndroid
On 2016/06/20 17:24:22, David Trainor wrote:
> Should we just query this through blimp_contents_impl_?

I'm not sure that makes sense. This is not a raw pointer, but instead this is
the BlimpContentsImplAndroid that owns the BlimpNavigationControllerImplAndroid.
This setup is similar to what we have in //content.

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/andro...
File
blimp/client/core/android/java/src/org/chromium/blimp/core/BlimpContentsImpl.java
(right):

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/andro...
blimp/client/core/android/java/src/org/chromium/blimp/core/BlimpContentsImpl.java:78:
nativeDestroy(mNativeBlimpContentsImplAndroid);
On 2016/06/20 17:24:22, David Trainor wrote:
> assert mNativeBlimpContentsImplAndroid != 0?

Done.

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/blimp...
File blimp/client/core/blimp_contents_impl.cc (right):

https://codereview.chromium.org/2059443002/diff/40001/blimp/client/core/blimp...
blimp/client/core/blimp_contents_impl.cc:33: delete blimp_contents_impl_android;
On 2016/06/20 17:24:22, David Trainor wrote:
> Will this crash?  Doesn't the underlying map actually own the user data?

Yes it will. :-( Updated the Java test code to ensure that the BlimpContents is
correctly destroyed as well, so now the test catches this crash.

Also, of course removed this code as well now :-)

[nyquist, 2016-06-21 23:01:11]

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

[commit-bot: I haz the power, 2016-06-21 23:02:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2059443002/100001

[David Trainor- moved to gerrit, 2016-06-21 23:12:54]

lgtm!

[nyquist, 2016-06-21 23:28:50]

The CQ bit was unchecked by nyquist@chromium.org

[nyquist, 2016-06-21 23:28:58]

The CQ bit was checked by nyquist@chromium.org

[commit-bot: I haz the power, 2016-06-21 23:29:56]

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

[commit-bot: I haz the power, 2016-06-22 02:23:25]

Description was changed from

==========
Add BlimpContents[,Observer] and BlimpNavigationController

This CL adds the public API for //blimp/client/core and adds a simple
implementation with a test for the core functionality for Java.

The setup of BlimpContents and related classes is intentionally
similar to how the Java version of the //content API is organized.

Users of this API are supposed to depend on the code in
//blimp/client/core/public, and not the implementation which lives
in //blimp/client/core directly.

An extra class compared to the C++ version is added for creating
BlimpContents, the BlimpContentsFactory. This sets up all the pointers
and references in the same ways as a BlimpContents created in native
code, but adds the requirement for Java code to invoke the destroy()
method of a BlimpContents.

BUG=611103
==========

to

==========
Add BlimpContents[,Observer] and BlimpNavigationController

This CL adds the public API for //blimp/client/core and adds a simple
implementation with a test for the core functionality for Java.

The setup of BlimpContents and related classes is intentionally
similar to how the Java version of the //content API is organized.

Users of this API are supposed to depend on the code in
//blimp/client/core/public, and not the implementation which lives
in //blimp/client/core directly.

An extra class compared to the C++ version is added for creating
BlimpContents, the BlimpContentsFactory. This sets up all the pointers
and references in the same ways as a BlimpContents created in native
code, but adds the requirement for Java code to invoke the destroy()
method of a BlimpContents.

BUG=611103
==========

[commit-bot: I haz the power, 2016-06-22 02:23:27]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-06-22 02:26:12]

Description was changed from

==========
Add BlimpContents[,Observer] and BlimpNavigationController

This CL adds the public API for //blimp/client/core and adds a simple
implementation with a test for the core functionality for Java.

The setup of BlimpContents and related classes is intentionally
similar to how the Java version of the //content API is organized.

Users of this API are supposed to depend on the code in
//blimp/client/core/public, and not the implementation which lives
in //blimp/client/core directly.

An extra class compared to the C++ version is added for creating
BlimpContents, the BlimpContentsFactory. This sets up all the pointers
and references in the same ways as a BlimpContents created in native
code, but adds the requirement for Java code to invoke the destroy()
method of a BlimpContents.

BUG=611103
==========

to

==========
Add BlimpContents[,Observer] and BlimpNavigationController

This CL adds the public API for //blimp/client/core and adds a simple
implementation with a test for the core functionality for Java.

The setup of BlimpContents and related classes is intentionally
similar to how the Java version of the //content API is organized.

Users of this API are supposed to depend on the code in
//blimp/client/core/public, and not the implementation which lives
in //blimp/client/core directly.

An extra class compared to the C++ version is added for creating
BlimpContents, the BlimpContentsFactory. This sets up all the pointers
and references in the same ways as a BlimpContents created in native
code, but adds the requirement for Java code to invoke the destroy()
method of a BlimpContents.

BUG=611103

Committed: https://crrev.com/986f860c8fc6361d727a9ccd3c8cd44727b6f5ca
Cr-Commit-Position: refs/heads/master@{#401170}
==========

[commit-bot: I haz the power, 2016-06-22 02:26:14]

Patchset 6 (id:??) landed as
https://crrev.com/986f860c8fc6361d727a9ccd3c8cd44727b6f5ca
Cr-Commit-Position: refs/heads/master@{#401170}