Make //blimp/client/app a real embedder of //blimp/client/public

Make //blimp/client/app a real embedder of //blimp/client/public

While making //chrome embed //blimp using //blimp/client/public, the
Blimp shell APK was kept compiling and working, but still using the
internal APIs of the new code in //blimp/client/core.

This CL updates the Blimp shell APK code in //blimp/client/app to
use the new public API, which was the initial goal to eventually do.

As part of this, a new class was added to store all the member
global members that need to be kept alive; the BlimpEnvironment.

In addition, a new delegate for the BlimpClientContext was created,
that basically just logs everything all messages, and uses the
embedder support library to provide functionality like an
IdentityProvider.

The business logic changes mainly happen in BlimpRendererActivity,
the toolbar, and the BlimpContentsDisplay, in addition to a
the new preferences framework which mimics the one from //chrome.

This made it possible to delete a lot of old code and resources,
which is also done in this CL, in addition to clean up all
unnecessary dependencies and visibility rules.

The BlimpClientSession and related test code was also moved to the
engine browser tests.

Since the enable_blimp_client GN argument is still around,
//blimp/client/public still does not link in the non-dummy core
code yet, so there are still direct dependencies on core from the
app directory. This may easily be removed if enable_blimp_client
is discontinued, and the public API links in the right code.

BUG=651964
Committed: https://crrev.com/a1fc2100ab3291b7a5852786ae55be932b688cce
Cr-Commit-Position: refs/heads/master@{#436526}

[nyquist, 2016-12-01 21:25:31]

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

[commit-bot: I haz the power, 2016-12-01 21:26: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, 2016-12-01 21:33:18]

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

[commit-bot: I haz the power, 2016-12-01 21:33:19]

Dry run: Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[nyquist, 2016-12-02 00:15:59]

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

[commit-bot: I haz the power, 2016-12-02 00:16:33]

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-12-02 02:46:45]

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

[commit-bot: I haz the power, 2016-12-02 02:46:46]

Dry run: This issue passed the CQ dry run.

[nyquist, 2016-12-02 07:09:52]

Description was changed from

==========
Make //blimp/client/app a real embedder of //blimp/client/public

BUG=
==========

to

==========
Make //blimp/client/app a real embedder of //blimp/client/public

While making //chrome embed //blimp using //blimp/client/public, the
Blimp shell APK was kept compiling and working, but still using the
internal APIs of the new code in //blimp/client/core.

This CL updates the Blimp shell APK code in //blimp/client/app to
use the new public API, which was the initial goal to eventually do.

As part of this, a new class was added to store all the member
global members that need to be kept alive; the BlimpEnvironment.

In addition, a new delegate for the BlimpClientContext was created,
that basically just logs everything all messages, and uses the
embedder support library to provide functionality like an
IdentityProvider.

The business logic changes mainly happen in BlimpRendererActivity,
the toolbar, and the BlimpContentsDisplay, in addition to a
the new preferences framework which mimics the one from //chrome.

This made it possible to delete a lot of old code and resources,
which is also done in this CL, in addition to clean up all
unnecessary dependencies and visibility rules.

The BlimpClientSession and related test code was also moved to the
engine browser tests.

Since the enable_blimp_client GN argument is still around,
//blimp/client/public still does not link in the non-dummy core
code yet, so there are still direct dependencies on core from the
app directory. This may easily be removed if enable_blimp_client
is discontinued, and the public API links in the right code.


BUG=
==========

[nyquist, 2016-12-02 07:10:35]

Description was changed from

==========
Make //blimp/client/app a real embedder of //blimp/client/public

While making //chrome embed //blimp using //blimp/client/public, the
Blimp shell APK was kept compiling and working, but still using the
internal APIs of the new code in //blimp/client/core.

This CL updates the Blimp shell APK code in //blimp/client/app to
use the new public API, which was the initial goal to eventually do.

As part of this, a new class was added to store all the member
global members that need to be kept alive; the BlimpEnvironment.

In addition, a new delegate for the BlimpClientContext was created,
that basically just logs everything all messages, and uses the
embedder support library to provide functionality like an
IdentityProvider.

The business logic changes mainly happen in BlimpRendererActivity,
the toolbar, and the BlimpContentsDisplay, in addition to a
the new preferences framework which mimics the one from //chrome.

This made it possible to delete a lot of old code and resources,
which is also done in this CL, in addition to clean up all
unnecessary dependencies and visibility rules.

The BlimpClientSession and related test code was also moved to the
engine browser tests.

Since the enable_blimp_client GN argument is still around,
//blimp/client/public still does not link in the non-dummy core
code yet, so there are still direct dependencies on core from the
app directory. This may easily be removed if enable_blimp_client
is discontinued, and the public API links in the right code.


BUG=
==========

to

==========
Make //blimp/client/app a real embedder of //blimp/client/public

While making //chrome embed //blimp using //blimp/client/public, the
Blimp shell APK was kept compiling and working, but still using the
internal APIs of the new code in //blimp/client/core.

This CL updates the Blimp shell APK code in //blimp/client/app to
use the new public API, which was the initial goal to eventually do.

As part of this, a new class was added to store all the member
global members that need to be kept alive; the BlimpEnvironment.

In addition, a new delegate for the BlimpClientContext was created,
that basically just logs everything all messages, and uses the
embedder support library to provide functionality like an
IdentityProvider.

The business logic changes mainly happen in BlimpRendererActivity,
the toolbar, and the BlimpContentsDisplay, in addition to a
the new preferences framework which mimics the one from //chrome.

This made it possible to delete a lot of old code and resources,
which is also done in this CL, in addition to clean up all
unnecessary dependencies and visibility rules.

The BlimpClientSession and related test code was also moved to the
engine browser tests.

Since the enable_blimp_client GN argument is still around,
//blimp/client/public still does not link in the non-dummy core
code yet, so there are still direct dependencies on core from the
app directory. This may easily be removed if enable_blimp_client
is discontinued, and the public API links in the right code.


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

[nyquist, 2016-12-02 07:11:00]

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

[nyquist, 2016-12-02 07:11:01]

dtrainor: PTAL

[David Trainor- moved to gerrit, 2016-12-02 18:16:14]

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/BUILD.gn
File blimp/client/app/BUILD.gn (right):

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/BUILD....
blimp/client/app/BUILD.gn:94: "//blimp/client/core",  # Necessary to link in
correct code.
Boo!

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
File blimp/client/app/android/blimp_environment.cc (right):

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
blimp/client/app/android/blimp_environment.cc:34:
BlimpClientContext::ApplyBlimpSwitches(this);
Register command line overrides?

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
File
blimp/client/app/android/java/src/org/chromium/blimp/app/BlimpEnvironment.java
(right):

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/app/BlimpEnvironment.java:33:
public void destroy() {
Should this clear the static?

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
File
blimp/client/app/android/java/src/org/chromium/blimp/app/toolbar/Toolbar.java
(right):

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/app/toolbar/Toolbar.java:66:
mIsInitialized = true;
Can we just check if mBlimpContents is null?

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/app/toolbar/Toolbar.java:74:
// something something about connection.addObserver(mToolbarMenu);
?

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/app/toolbar/Toolbar.java:99:
mBlimpContents.getNavigationController().goBack();
Check mBlimpContents all places?

[nyquist, 2016-12-02 22:35:47]

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/BUILD.gn
File blimp/client/app/BUILD.gn (right):

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/BUILD....
blimp/client/app/BUILD.gn:94: "//blimp/client/core",  # Necessary to link in
correct code.
On 2016/12/02 18:16:14, David Trainor wrote:
> Boo!

Yeah; until enable_blimp_client is gone :-/

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
File blimp/client/app/android/blimp_environment.cc (right):

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
blimp/client/app/android/blimp_environment.cc:34:
BlimpClientContext::ApplyBlimpSwitches(this);
On 2016/12/02 18:16:14, David Trainor wrote:
> Register command line overrides?

Not 100% what you mean by this.
BlimpClientContext::ApplyBlimpSwitches(...) calls
Settings::ApplyBlimpSwitches().

Similarly, BlimpClientContext::RegisterPrefs(...) calls
Settings::RegisterPrefs().

See
https://cs.chromium.org/chromium/src/blimp/client/core/context/blimp_client_c...

Happy to fix this as soon as I understand what you meant :-)

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
File
blimp/client/app/android/java/src/org/chromium/blimp/app/BlimpEnvironment.java
(right):

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/app/BlimpEnvironment.java:33:
public void destroy() {
On 2016/12/02 18:16:14, David Trainor wrote:
> Should this clear the static?

Done.

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
File
blimp/client/app/android/java/src/org/chromium/blimp/app/toolbar/Toolbar.java
(right):

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/app/toolbar/Toolbar.java:66:
mIsInitialized = true;
On 2016/12/02 18:16:14, David Trainor wrote:
> Can we just check if mBlimpContents is null?

Done.

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/app/toolbar/Toolbar.java:74:
// something something about connection.addObserver(mToolbarMenu);
On 2016/12/02 18:16:14, David Trainor wrote:
> ?

Done.

https://codereview.chromium.org/2542083004/diff/20001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/app/toolbar/Toolbar.java:99:
mBlimpContents.getNavigationController().goBack();
On 2016/12/02 18:16:14, David Trainor wrote:
> Check mBlimpContents all places?

Done.

[nyquist, 2016-12-02 22:42:19]

nyquist@chromium.org changed reviewers:
+ pam@chromium.org, rogerta@chromium.org

[nyquist, 2016-12-02 22:42:20]

pam: PTAL new DEPS on //components/prefs and //components/pref_registry in
//blimp/client/app/android/DEPS.
Usage in //blimp/client/app/android/blimp_environment.cc, mostly copied from
//blimp/client/app/linux/blimp_main.cc.

rogerta: PTAL new DEPS on //components/signin in
//blimp/client/app/android/DEPS.
Usage in //blimp/client/app/android/blimp_client_context_delegate_android.cc,
mostly copied from
//blimp/client/app/linux/blimp_client_context_delegate_linux.cc.

[Roger Tawa OOO till Jul 10th, 2016-12-05 16:36:37]

lgtm deps on components/signin

[David Trainor- moved to gerrit, 2016-12-05 18:51:18]

lgtm

[nyquist, 2016-12-05 21:45:42]

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

[nyquist, 2016-12-05 21:45:43]

gab: PTAL new DEPS on //components/prefs and //components/pref_registry in
//blimp/client/app/android/DEPS.
Usage in //blimp/client/app/android/blimp_environment.cc, mostly copied from
//blimp/client/app/linux/blimp_main.cc.

[gab, 2016-12-05 22:00:23]

On 2016/12/05 21:45:43, nyquist wrote:
> gab: PTAL new DEPS on //components/prefs and //components/pref_registry in
> //blimp/client/app/android/DEPS.
> Usage in //blimp/client/app/android/blimp_environment.cc, mostly copied from
> //blimp/client/app/linux/blimp_main.cc.

lgtm

[nyquist, 2016-12-05 22:06:29]

nyquist@chromium.org changed reviewers:
- pam@chromium.org

[nyquist, 2016-12-05 22:07:21]

The CQ bit was checked by nyquist@chromium.org

[commit-bot: I haz the power, 2016-12-05 22:08:12]

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

[nyquist, 2016-12-05 23:59:00]

The CQ bit was checked by nyquist@chromium.org

[nyquist, 2016-12-05 23:59:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, rogerta@chromium.org, gab@chromium.org
Link to the patchset: https://codereview.chromium.org/2542083004/#ps60001
(title: "Fix findbugs issue")

[commit-bot: I haz the power, 2016-12-05 23:59:29]

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-12-06 01:32:43]

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

[commit-bot: I haz the power, 2016-12-06 01:32:44]

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

[nyquist, 2016-12-06 03:47:21]

The CQ bit was checked by nyquist@chromium.org

[commit-bot: I haz the power, 2016-12-06 03: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-12-06 04:36:45]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1480996041856280,
"parent_rev": "ea55d5ac6434dbd26017c1ecff10b7972e6c04d3", "commit_rev":
"f6d82bcaef3f2954e47a3fc830cefdd935ab7c21"}

[commit-bot: I haz the power, 2016-12-06 04:37:09]

Description was changed from

==========
Make //blimp/client/app a real embedder of //blimp/client/public

While making //chrome embed //blimp using //blimp/client/public, the
Blimp shell APK was kept compiling and working, but still using the
internal APIs of the new code in //blimp/client/core.

This CL updates the Blimp shell APK code in //blimp/client/app to
use the new public API, which was the initial goal to eventually do.

As part of this, a new class was added to store all the member
global members that need to be kept alive; the BlimpEnvironment.

In addition, a new delegate for the BlimpClientContext was created,
that basically just logs everything all messages, and uses the
embedder support library to provide functionality like an
IdentityProvider.

The business logic changes mainly happen in BlimpRendererActivity,
the toolbar, and the BlimpContentsDisplay, in addition to a
the new preferences framework which mimics the one from //chrome.

This made it possible to delete a lot of old code and resources,
which is also done in this CL, in addition to clean up all
unnecessary dependencies and visibility rules.

The BlimpClientSession and related test code was also moved to the
engine browser tests.

Since the enable_blimp_client GN argument is still around,
//blimp/client/public still does not link in the non-dummy core
code yet, so there are still direct dependencies on core from the
app directory. This may easily be removed if enable_blimp_client
is discontinued, and the public API links in the right code.


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

to

==========
Make //blimp/client/app a real embedder of //blimp/client/public

While making //chrome embed //blimp using //blimp/client/public, the
Blimp shell APK was kept compiling and working, but still using the
internal APIs of the new code in //blimp/client/core.

This CL updates the Blimp shell APK code in //blimp/client/app to
use the new public API, which was the initial goal to eventually do.

As part of this, a new class was added to store all the member
global members that need to be kept alive; the BlimpEnvironment.

In addition, a new delegate for the BlimpClientContext was created,
that basically just logs everything all messages, and uses the
embedder support library to provide functionality like an
IdentityProvider.

The business logic changes mainly happen in BlimpRendererActivity,
the toolbar, and the BlimpContentsDisplay, in addition to a
the new preferences framework which mimics the one from //chrome.

This made it possible to delete a lot of old code and resources,
which is also done in this CL, in addition to clean up all
unnecessary dependencies and visibility rules.

The BlimpClientSession and related test code was also moved to the
engine browser tests.

Since the enable_blimp_client GN argument is still around,
//blimp/client/public still does not link in the non-dummy core
code yet, so there are still direct dependencies on core from the
app directory. This may easily be removed if enable_blimp_client
is discontinued, and the public API links in the right code.


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

[commit-bot: I haz the power, 2016-12-06 04:37:11]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-06 04:41:44]

Description was changed from

==========
Make //blimp/client/app a real embedder of //blimp/client/public

While making //chrome embed //blimp using //blimp/client/public, the
Blimp shell APK was kept compiling and working, but still using the
internal APIs of the new code in //blimp/client/core.

This CL updates the Blimp shell APK code in //blimp/client/app to
use the new public API, which was the initial goal to eventually do.

As part of this, a new class was added to store all the member
global members that need to be kept alive; the BlimpEnvironment.

In addition, a new delegate for the BlimpClientContext was created,
that basically just logs everything all messages, and uses the
embedder support library to provide functionality like an
IdentityProvider.

The business logic changes mainly happen in BlimpRendererActivity,
the toolbar, and the BlimpContentsDisplay, in addition to a
the new preferences framework which mimics the one from //chrome.

This made it possible to delete a lot of old code and resources,
which is also done in this CL, in addition to clean up all
unnecessary dependencies and visibility rules.

The BlimpClientSession and related test code was also moved to the
engine browser tests.

Since the enable_blimp_client GN argument is still around,
//blimp/client/public still does not link in the non-dummy core
code yet, so there are still direct dependencies on core from the
app directory. This may easily be removed if enable_blimp_client
is discontinued, and the public API links in the right code.


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

to

==========
Make //blimp/client/app a real embedder of //blimp/client/public

While making //chrome embed //blimp using //blimp/client/public, the
Blimp shell APK was kept compiling and working, but still using the
internal APIs of the new code in //blimp/client/core.

This CL updates the Blimp shell APK code in //blimp/client/app to
use the new public API, which was the initial goal to eventually do.

As part of this, a new class was added to store all the member
global members that need to be kept alive; the BlimpEnvironment.

In addition, a new delegate for the BlimpClientContext was created,
that basically just logs everything all messages, and uses the
embedder support library to provide functionality like an
IdentityProvider.

The business logic changes mainly happen in BlimpRendererActivity,
the toolbar, and the BlimpContentsDisplay, in addition to a
the new preferences framework which mimics the one from //chrome.

This made it possible to delete a lot of old code and resources,
which is also done in this CL, in addition to clean up all
unnecessary dependencies and visibility rules.

The BlimpClientSession and related test code was also moved to the
engine browser tests.

Since the enable_blimp_client GN argument is still around,
//blimp/client/public still does not link in the non-dummy core
code yet, so there are still direct dependencies on core from the
app directory. This may easily be removed if enable_blimp_client
is discontinued, and the public API links in the right code.

BUG=651964

Committed: https://crrev.com/a1fc2100ab3291b7a5852786ae55be932b688cce
Cr-Commit-Position: refs/heads/master@{#436526}
==========

[commit-bot: I haz the power, 2016-12-06 04:41:46]

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