Refactor Chromoting JNI code to use jni/Client (Java changes only).

Reland: Refactor Chromoting JNI code to use jni/Client (Java changes only).

CL was reverted as the FindBugs step broke on an internal
builder. Relanding with FB suppression.

Original CL description:
This removes globals from jni/JniInterface in favor of passing a
jni/Client instance to code that needs to access the Chromoting
client connection.

The C++ code is unchanged - JniInterface class is still used to
marshall Java <-> C++ calls. JniInterface still holds a singleton
Client connection, but all other globals have been removed (including
the singleton CapabilityManager instance).

Each Activity's onCreate() method gets the singleton Client instance,
and then passes it to objects that need it (for input injection and
so on). This should make it easier to use a fake Client instance for
unit-instrumentation tests.

BUG=526336, 585799
Committed: https://crrev.com/40e8dff71ae626f479e7356515d7bfce0b9b68b0
Cr-Commit-Position: refs/heads/master@{#374924}

[Lambros, 2015-12-19 03:15:50]

lambroslambrou@chromium.org changed reviewers:
+ joedow@chromium.org, sergeyu@chromium.org

[Lambros, 2015-12-19 03:15:51]

sergeyu: Overall review
joedow: Anything that impacts Touch/Tracking mode. Please could you test to make
sure that Touch still works as expected with the CapabilityManager?

[Sergey Ulanov, 2015-12-21 17:58:15]

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/Chromoting.java (right):

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Chromoting.java:434: Client
client = JniInterface.createClient();
Can this be replaced with "new Client()". The Client's constructor should be
responsible for setting the global reference in JniInstance.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Chromoting.java:444:
JniInterface.destroyClient();
client.destroy()?

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/SessionAuthenticator.java
(right):

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/SessionAuthenticator.java:86:
JniInterface.destroyClient();
can this be replaced with a call to mClient? i.e. call mClient.destroy() here
and then the Client will be responsible for resetting the static reference.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/jni/Client.java (right):

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:26: public
class Client {
maybe not for this CL: define abstract interface for Client to make it easy to
mock.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:145: *
Performs the native response to the user's PIN.
Reword this. E.g. "Called by mAuthenticator after the user enters the PIN."

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:158: * Called
from native code, to save newly-received pairing credentials to permanent
storage.
many of these comments need to be rewritten. E.g. this function is called from
JniInstance, not from native code.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java
(right):

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java:52:
public static Client getClient() {
These look out of place here. Move to the Client class? And then JniInstance
will be using Client.getInstance()?

[joedow, 2016-01-19 16:35:06]

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/jni/Client.java (right):

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:48: /**
Whether the native code is attempting a connection. Accessed on the UI thread.
*/
Should this be 'attempting' a connection or 'established' a connection?  The var
name is mConnected which seems like it has already made one.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:105:
ConnectionListener.State.CLOSED, ConnectionListener.Error.OK);
Should this event get fired after the connection was actually disconnected?

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:107:
disconnectFromHostWithoutNotification();
It looks like 'onConnectionState()' already has this logic, should you call that
method instead?

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:187: * Presses
or releases the specified (nonnegative) key. Called on the UI thread. If
scanCode
Does it make sense to check the keypress value to see if it is 'nonnegative'?

[Lambros, 2016-01-29 23:58:26]

ptal

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/Chromoting.java (right):

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Chromoting.java:434: Client
client = JniInterface.createClient();
On 2015/12/21 17:58:15, Sergey Ulanov wrote:
> Can this be replaced with "new Client()". The Client's constructor should be
> responsible for setting the global reference in JniInstance. 

Done.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Chromoting.java:444:
JniInterface.destroyClient();
On 2015/12/21 17:58:15, Sergey Ulanov wrote:
> client.destroy()?

Done.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/SessionAuthenticator.java
(right):

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/SessionAuthenticator.java:86:
JniInterface.destroyClient();
On 2015/12/21 17:58:15, Sergey Ulanov wrote:
> can this be replaced with a call to mClient? i.e. call mClient.destroy() here
> and then the Client will be responsible for resetting the static reference.

Done.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/jni/Client.java (right):

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:26: public
class Client {
On 2015/12/21 17:58:15, Sergey Ulanov wrote:
> maybe not for this CL: define abstract interface for Client to make it easy to
> mock.

Acknowledged.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:48: /**
Whether the native code is attempting a connection. Accessed on the UI thread.
*/
On 2016/01/19 16:35:06, joedow wrote:
> Should this be 'attempting' a connection or 'established' a connection?  The
var
> name is mConnected which seems like it has already made one.

I'd prefer to clean this up in a followup CL - I might be able to get rid of it
eventually. The flag seems to mean "do we have a C++
ChromotingJniRuntime::session() object?" That object is created in
nativeConnect() and destroyed in nativeDisconnect().

The flag guards against calling various native methods (such as sendKeyEvent())
outside of a "connected" session, deferencing the null session() object.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:105:
ConnectionListener.State.CLOSED, ConnectionListener.Error.OK);
On 2016/01/19 16:35:06, joedow wrote:
> Should this event get fired after the connection was actually disconnected?

I'm not certain that changing this won't break anything. I'd prefer to leave it
as-is in this refactoring CL.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:107:
disconnectFromHostWithoutNotification();
On 2016/01/19 16:35:06, joedow wrote:
> It looks like 'onConnectionState()' already has this logic, should you call
that
> method instead?

I don't follow?
This code looks fine to me, it's just copied from JniInterface.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:145: *
Performs the native response to the user's PIN.
On 2015/12/21 17:58:15, Sergey Ulanov wrote:
> Reword this. E.g. "Called by mAuthenticator after the user enters the PIN."

Done.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:158: * Called
from native code, to save newly-received pairing credentials to permanent
storage.
On 2015/12/21 17:58:15, Sergey Ulanov wrote:
> many of these comments need to be rewritten. E.g. this function is called from
> JniInstance, not from native code.

Done.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:187: * Presses
or releases the specified (nonnegative) key. Called on the UI thread. If
scanCode
On 2016/01/19 16:35:06, joedow wrote:
> Does it make sense to check the keypress value to see if it is 'nonnegative'?

I'm not sure why "nonnegative" appears here - I'll remove it. It's ambiguous now
there are 2 int parameters.
Some checking is done anyway in
remoting::ChromotingJniRuntime::GetInstance()->session() so there shouldn't be
any need for more checks.

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java
(right):

https://codereview.chromium.org/1537183002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java:52:
public static Client getClient() {
On 2015/12/21 17:58:15, Sergey Ulanov wrote:
> These look out of place here. Move to the Client class? And then JniInstance
> will be using Client.getInstance()?

Done.

[Sergey Ulanov, 2016-01-30 01:03:24]

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
File remoting/android/java/src/org/chromium/chromoting/Chromoting.java (right):

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/Chromoting.java:341:
Client.getClient().destroy();
keep the Client object created in connectToHost() (i.e. store it as mClient) and
use it here instead of calling getClient(). The fact that that Client is a
singleton should only matter in JniInterface

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
File remoting/android/java/src/org/chromium/chromoting/Desktop.java (right):

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:104: mClient =
Client.getClient();
Is it possible to pass the Client object from the Chromoting activity here
instead of calling Client.getClient()?

This would not work if we wanted to support multiple concurrent connections.

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
File remoting/android/java/src/org/chromium/chromoting/jni/Client.java (right):

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:60: public
static Client getClient() {
call it getInstance()?

[Lambros, 2016-02-03 22:57:43]

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
File remoting/android/java/src/org/chromium/chromoting/Chromoting.java (right):

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/Chromoting.java:341:
Client.getClient().destroy();
On 2016/01/30 01:03:23, Sergey Ulanov wrote:
> keep the Client object created in connectToHost() (i.e. store it as mClient)
and
> use it here instead of calling getClient(). The fact that that Client is a
> singleton should only matter in JniInterface

I'm not sure we should be disconnecting anyway in Chromoting.onDestroy(). It
should be fine for the OS to kill the Chromoting activity and keep the Desktop
activity alive.
But in the interest of keeping this as a refactoring CL without changing
behavior, I've added mClient and a TODO.

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
File remoting/android/java/src/org/chromium/chromoting/Desktop.java (right):

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:104: mClient =
Client.getClient();
On 2016/01/30 01:03:23, Sergey Ulanov wrote:
> Is it possible to pass the Client object from the Chromoting activity here
> instead of calling Client.getClient()?
> 
> This would not work if we wanted to support multiple concurrent connections.

This is the reason we need the singleton in the first place! Activities cannot
directly pass Java objects to other Activities. You're meant to
serialize/deserialize and communicate via Intents. However, we abuse the fact
that Android hosts the activities in the same VM instance, and use the global
singleton to "cheat". (A similar scheme is used to pass the screenshot bitmap to
the HelpActivity)

For multiple connections, we'd probably have a global mapping of ID->Client, and
pass the string ID to the new Desktop activity via an Intent. So this onCreate()
method would become something like:
Intent intent = getIntent();
String id = ... extract ID from intent ...
mClient = Client.getForId(id);

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
File remoting/android/java/src/org/chromium/chromoting/jni/Client.java (right):

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:60: public
static Client getClient() {
On 2016/01/30 01:03:23, Sergey Ulanov wrote:
> call it getInstance()?

Done.

[Sergey Ulanov, 2016-02-08 22:05:16]

lgtm

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
File remoting/android/java/src/org/chromium/chromoting/Desktop.java (right):

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:104: mClient =
Client.getClient();
On 2016/02/03 22:57:43, Lambros wrote:
> On 2016/01/30 01:03:23, Sergey Ulanov wrote:
> > Is it possible to pass the Client object from the Chromoting activity here
> > instead of calling Client.getClient()?
> > 
> > This would not work if we wanted to support multiple concurrent connections.
> 
> This is the reason we need the singleton in the first place! Activities cannot
> directly pass Java objects to other Activities. You're meant to
> serialize/deserialize and communicate via Intents. However, we abuse the fact
> that Android hosts the activities in the same VM instance, and use the global
> singleton to "cheat". (A similar scheme is used to pass the screenshot bitmap
to
> the HelpActivity)
> 
> For multiple connections, we'd probably have a global mapping of ID->Client,
and
> pass the string ID to the new Desktop activity via an Intent. So this
onCreate()
> method would become something like:
> Intent intent = getIntent();
> String id = ... extract ID from intent ...
> mClient = Client.getForId(id);
> 

I see. I think the right way to fix this would be to create Desktop activity as
soon as user taps on a host and make the Desktop activity responsible for
connecting the client.

[Lambros, 2016-02-08 23:48:51]

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
File remoting/android/java/src/org/chromium/chromoting/Desktop.java (right):

https://codereview.chromium.org/1537183002/diff/60001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:104: mClient =
Client.getClient();
On 2016/02/08 22:05:16, Sergey Ulanov wrote:
> On 2016/02/03 22:57:43, Lambros wrote:
> > On 2016/01/30 01:03:23, Sergey Ulanov wrote:
> > > Is it possible to pass the Client object from the Chromoting activity here
> > > instead of calling Client.getClient()?
> > > 
> > > This would not work if we wanted to support multiple concurrent
connections.
> > 
> > This is the reason we need the singleton in the first place! Activities
cannot
> > directly pass Java objects to other Activities. You're meant to
> > serialize/deserialize and communicate via Intents. However, we abuse the
fact
> > that Android hosts the activities in the same VM instance, and use the
global
> > singleton to "cheat". (A similar scheme is used to pass the screenshot
bitmap
> to
> > the HelpActivity)
> > 
> > For multiple connections, we'd probably have a global mapping of ID->Client,
> and
> > pass the string ID to the new Desktop activity via an Intent. So this
> onCreate()
> > method would become something like:
> > Intent intent = getIntent();
> > String id = ... extract ID from intent ...
> > mClient = Client.getForId(id);
> > 
> 
> I see. I think the right way to fix this would be to create Desktop activity
as
> soon as user taps on a host and make the Desktop activity responsible for
> connecting the client.

That would work for the initial connection, but Android can restart the Desktop
activity (for example, when rotating the device or connecting a BT keyboard), so
you'd have to recreate the connection somehow. An Activity gets a chance to
save/restore its state, but I think [de]serialization is still required.

[Lambros, 2016-02-09 02:12:17]

The CQ bit was checked by lambroslambrou@chromium.org

[commit-bot: I haz the power, 2016-02-09 02:19:00]

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

[commit-bot: I haz the power, 2016-02-09 03:12:16]

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

[commit-bot: I haz the power, 2016-02-09 03:12:17]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)

[Lambros, 2016-02-09 23:53:38]

The CQ bit was checked by lambroslambrou@chromium.org

[Lambros, 2016-02-09 23:53:38]

The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org
Link to the patchset: https://codereview.chromium.org/1537183002/#ps120001
(title: "fix tests")

[commit-bot: I haz the power, 2016-02-09 23:54:08]

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

[commit-bot: I haz the power, 2016-02-10 00:38:26]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-02-10 00:39:57]

Description was changed from

==========
Refactor Chromoting JNI code to use jni/Client (Java changes only).

This removes globals from jni/JniInterface in favor of passing a
jni/Client instance to code that needs to access the Chromoting
client connection.

The C++ code is unchanged - JniInterface class is still used to
marshall Java <-> C++ calls. JniInterface still holds a singleton
Client connection, but all other globals have been removed (including
the singleton CapabilityManager instance).

Each Activity's onCreate() method gets the singleton Client instance,
and then passes it to objects that need it (for input injection and
so on). This should make it easier to use a fake Client instance for
unit-instrumentation tests.

BUG=526336
==========

to

==========
Refactor Chromoting JNI code to use jni/Client (Java changes only).

This removes globals from jni/JniInterface in favor of passing a
jni/Client instance to code that needs to access the Chromoting
client connection.

The C++ code is unchanged - JniInterface class is still used to
marshall Java <-> C++ calls. JniInterface still holds a singleton
Client connection, but all other globals have been removed (including
the singleton CapabilityManager instance).

Each Activity's onCreate() method gets the singleton Client instance,
and then passes it to objects that need it (for input injection and
so on). This should make it easier to use a fake Client instance for
unit-instrumentation tests.

BUG=526336

Committed: https://crrev.com/22ee9b8426839b0c2c841fbc919cd4a79b1456d0
Cr-Commit-Position: refs/heads/master@{#374548}
==========

[commit-bot: I haz the power, 2016-02-10 00:39:58]

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

[hartmanng, 2016-02-10 14:59:08]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/1687873002/ by hartmanng@chromium.org.

The reason for reverting is: This broke ToT: http://crbug.com/585799.

[Lambros, 2016-02-11 01:13:58]

Description was changed from

==========
Refactor Chromoting JNI code to use jni/Client (Java changes only).

This removes globals from jni/JniInterface in favor of passing a
jni/Client instance to code that needs to access the Chromoting
client connection.

The C++ code is unchanged - JniInterface class is still used to
marshall Java <-> C++ calls. JniInterface still holds a singleton
Client connection, but all other globals have been removed (including
the singleton CapabilityManager instance).

Each Activity's onCreate() method gets the singleton Client instance,
and then passes it to objects that need it (for input injection and
so on). This should make it easier to use a fake Client instance for
unit-instrumentation tests.

BUG=526336

Committed: https://crrev.com/22ee9b8426839b0c2c841fbc919cd4a79b1456d0
Cr-Commit-Position: refs/heads/master@{#374548}
==========

to

==========
Reland: Refactor Chromoting JNI code to use jni/Client (Java changes only).

CL was reverted as the FindBugs step broke on an internal
builder. Relanding with FB suppression.

Original CL description:
This removes globals from jni/JniInterface in favor of passing a
jni/Client instance to code that needs to access the Chromoting
client connection.

The C++ code is unchanged - JniInterface class is still used to
marshall Java <-> C++ calls. JniInterface still holds a singleton
Client connection, but all other globals have been removed (including
the singleton CapabilityManager instance).

Each Activity's onCreate() method gets the singleton Client instance,
and then passes it to objects that need it (for input injection and
so on). This should make it easier to use a fake Client instance for
unit-instrumentation tests.

BUG=526336,585799
==========

[Lambros, 2016-02-11 01:15:39]

The CQ bit was checked by lambroslambrou@chromium.org

[Lambros, 2016-02-11 01:15:39]

The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org
Link to the patchset: https://codereview.chromium.org/1537183002/#ps140001
(title: "Suppress FindBugs warning")

[commit-bot: I haz the power, 2016-02-11 01:15:57]

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

[commit-bot: I haz the power, 2016-02-11 02:13:36]

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

[commit-bot: I haz the power, 2016-02-11 02:13:37]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)

[Lambros, 2016-02-11 17:56:55]

The CQ bit was checked by lambroslambrou@chromium.org

[commit-bot: I haz the power, 2016-02-11 17:57:21]

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

[commit-bot: I haz the power, 2016-02-11 19:39:26]

Description was changed from

==========
Reland: Refactor Chromoting JNI code to use jni/Client (Java changes only).

CL was reverted as the FindBugs step broke on an internal
builder. Relanding with FB suppression.

Original CL description:
This removes globals from jni/JniInterface in favor of passing a
jni/Client instance to code that needs to access the Chromoting
client connection.

The C++ code is unchanged - JniInterface class is still used to
marshall Java <-> C++ calls. JniInterface still holds a singleton
Client connection, but all other globals have been removed (including
the singleton CapabilityManager instance).

Each Activity's onCreate() method gets the singleton Client instance,
and then passes it to objects that need it (for input injection and
so on). This should make it easier to use a fake Client instance for
unit-instrumentation tests.

BUG=526336,585799
==========

to

==========
Reland: Refactor Chromoting JNI code to use jni/Client (Java changes only).

CL was reverted as the FindBugs step broke on an internal
builder. Relanding with FB suppression.

Original CL description:
This removes globals from jni/JniInterface in favor of passing a
jni/Client instance to code that needs to access the Chromoting
client connection.

The C++ code is unchanged - JniInterface class is still used to
marshall Java <-> C++ calls. JniInterface still holds a singleton
Client connection, but all other globals have been removed (including
the singleton CapabilityManager instance).

Each Activity's onCreate() method gets the singleton Client instance,
and then passes it to objects that need it (for input injection and
so on). This should make it easier to use a fake Client instance for
unit-instrumentation tests.

BUG=526336,585799
==========

[commit-bot: I haz the power, 2016-02-11 19:39:27]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-02-16 22:36:15]

Description was changed from

==========
Reland: Refactor Chromoting JNI code to use jni/Client (Java changes only).

CL was reverted as the FindBugs step broke on an internal
builder. Relanding with FB suppression.

Original CL description:
This removes globals from jni/JniInterface in favor of passing a
jni/Client instance to code that needs to access the Chromoting
client connection.

The C++ code is unchanged - JniInterface class is still used to
marshall Java <-> C++ calls. JniInterface still holds a singleton
Client connection, but all other globals have been removed (including
the singleton CapabilityManager instance).

Each Activity's onCreate() method gets the singleton Client instance,
and then passes it to objects that need it (for input injection and
so on). This should make it easier to use a fake Client instance for
unit-instrumentation tests.

BUG=526336,585799
==========

to

==========
Reland: Refactor Chromoting JNI code to use jni/Client (Java changes only).

CL was reverted as the FindBugs step broke on an internal
builder. Relanding with FB suppression.

Original CL description:
This removes globals from jni/JniInterface in favor of passing a
jni/Client instance to code that needs to access the Chromoting
client connection.

The C++ code is unchanged - JniInterface class is still used to
marshall Java <-> C++ calls. JniInterface still holds a singleton
Client connection, but all other globals have been removed (including
the singleton CapabilityManager instance).

Each Activity's onCreate() method gets the singleton Client instance,
and then passes it to objects that need it (for input injection and
so on). This should make it easier to use a fake Client instance for
unit-instrumentation tests.

BUG=526336,585799

Committed: https://crrev.com/40e8dff71ae626f479e7356515d7bfce0b9b68b0
Cr-Commit-Position: refs/heads/master@{#374924}
==========

[commit-bot: I haz the power, 2016-02-16 22:36:16]

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