[Android Client] Move session-scoped native interface into JniClient

[Android Client] Move session-scoped native interface into JniClient

This finishes up the refactorization of the JNI interface. Scheduled tasks
inside a session will not be lingering after the session ends.

BUG=526336
Committed: https://crrev.com/800991e54ced74fc425843070a1941827af5b787
Cr-Commit-Position: refs/heads/master@{#396036}

Committed: https://crrev.com/6a7a5a02baacc4b1ddaf58a22d03ffe723a263c7
Cr-Commit-Position: refs/heads/master@{#397209}

[Yuwei, 2016-05-25 18:07:01]

yuweih@chromium.org changed reviewers:
+ lambroslambrou@chromium.org

[Yuwei, 2016-05-25 18:07:01]

PTAL. I'll go back to CL 2007123003 after landing this CL. Thanks!

https://codereview.chromium.org/2007403002/diff/1/remoting/client/jni/jni_cli...
File remoting/client/jni/jni_client.h (right):

https://codereview.chromium.org/2007403002/diff/1/remoting/client/jni/jni_cli...
remoting/client/jni/jni_client.h:29: // Initiates a connection with the
specified host. Only call when a host
Just moved functions from ChormotingJniRuntime to here. Just slightly concern
about the loss of the git blame information...

[Yuwei, 2016-05-25 18:21:50]

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

[commit-bot: I haz the power, 2016-05-25 18:22:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2007403002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2007403002/1

[commit-bot: I haz the power, 2016-05-25 18:26:36]

Dry run: None

[Lambros, 2016-05-25 20:51:50]

lgtm

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

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:146: /**
Called by native code whenever the connection status changes. Called on the UI
thread. */
The CalledByNative annotation means you can remove "called by native code" from
the comment. This can be simply:
Called on the UI thread whenever the connection status changes.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:167:
@CalledByNative
No longer called by JniInterface. Comment can be simply:
Called on the UI thread to prompt the user to enter a PIN.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:189:
@CalledByNative
Saves newly-received ...

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:275: * Called
by JniInterface to perform the redrawing callback requested by
Performs the redrawing callback ...

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:305:
@CalledByNative
Sets a new video frame. ...

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:318: * and the
returned Bitmap is referenced by native code which writes the decoded frame
pixels
Remove "Called by JniInterface (from native code)"
"The returned bitmap is ..."

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:327: * Called
by JniInterface (from native code) to update the cursor shape. This is called on
the
Updates the cursor shape.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:355: * Called
by JniInterface (from native code), to pop up a third party login page to fetch
the
Pops up a third party ...

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:380: * Called
by JniInterface (from native code) to set the list of negotiated capabilities
between
Sets the list...

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:393: * Called
by JniInterface (from native code), to pass on the deconstructed
ExtensionMessage to
Passes on the ...

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:410: // JNI
Interface.
Remove this comment.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:412: private
native long nativeInit();
OK to group these native.. methods together, as long as you follow up with a CL
to do the same in JniInterface.

https://codereview.chromium.org/2007403002/diff/20001/remoting/client/jni/jni...
File remoting/client/jni/jni_client.h (right):

https://codereview.chromium.org/2007403002/diff/20001/remoting/client/jni/jni...
remoting/client/jni/jni_client.h:89: // JNI Interface.
Maybe remove this comment, or say instead "The following methods are exposed to
Java via JNI."

https://codereview.chromium.org/2007403002/diff/20001/remoting/client/jni/jni...
remoting/client/jni/jni_client.h:164: base::WeakPtr<JniClient> GetWeakPtr();
Optional, I don't mind either way:
Instead of exposing a weak pointer in the public API, I think our usual pattern
is to have Foo() and DoFoo() methods, where Foo() can be called on any thread,
and it trampolines to DoFoo() on the right thread. A lot of the time, Foo() and
DoFoo() are actually the same method.

https://codereview.chromium.org/2007403002/diff/20001/remoting/client/jni/jni...
remoting/client/jni/jni_client.h:173: // Pointer to the Java client object.
s/Pointer/Reference/

[Yuwei, 2016-05-25 21:17:34]

Patchset #3 (id:40001) has been deleted

[Yuwei, 2016-05-25 21:19:53]

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

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:146: /**
Called by native code whenever the connection status changes. Called on the UI
thread. */
On 2016/05/25 20:51:49, Lambros wrote:
> The CalledByNative annotation means you can remove "called by native code"
from
> the comment. This can be simply:
> Called on the UI thread whenever the connection status changes.

Done.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:167:
@CalledByNative
On 2016/05/25 20:51:50, Lambros wrote:
> No longer called by JniInterface. Comment can be simply:
> Called on the UI thread to prompt the user to enter a PIN.

Done.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:189:
@CalledByNative
On 2016/05/25 20:51:50, Lambros wrote:
> Saves newly-received ...

Done.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:275: * Called
by JniInterface to perform the redrawing callback requested by
On 2016/05/25 20:51:50, Lambros wrote:
> Performs the redrawing callback ...

Done.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:305:
@CalledByNative
On 2016/05/25 20:51:49, Lambros wrote:
> Sets a new video frame. ...

Done.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:318: * and the
returned Bitmap is referenced by native code which writes the decoded frame
pixels
On 2016/05/25 20:51:49, Lambros wrote:
> Remove "Called by JniInterface (from native code)"
> "The returned bitmap is ..."

Done.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:327: * Called
by JniInterface (from native code) to update the cursor shape. This is called on
the
On 2016/05/25 20:51:50, Lambros wrote:
> Updates the cursor shape.

Done.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:355: * Called
by JniInterface (from native code), to pop up a third party login page to fetch
the
On 2016/05/25 20:51:49, Lambros wrote:
> Pops up a third party ...

Done.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:380: * Called
by JniInterface (from native code) to set the list of negotiated capabilities
between
On 2016/05/25 20:51:50, Lambros wrote:
> Sets the list...

Done.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:393: * Called
by JniInterface (from native code), to pass on the deconstructed
ExtensionMessage to
On 2016/05/25 20:51:50, Lambros wrote:
> Passes on the ...

Done.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:410: // JNI
Interface.
On 2016/05/25 20:51:49, Lambros wrote:
> Remove this comment.

Done.

https://codereview.chromium.org/2007403002/diff/20001/remoting/android/java/s...
remoting/android/java/src/org/chromium/chromoting/jni/Client.java:412: private
native long nativeInit();
On 2016/05/25 20:51:49, Lambros wrote:
> OK to group these native.. methods together, as long as you follow up with a
CL
> to do the same in JniInterface.
> 

Since it is just moving one line of code (i.e. private static native void
nativeLoadNative()), I think it is okay to do it in this CL?

https://codereview.chromium.org/2007403002/diff/20001/remoting/client/jni/jni...
File remoting/client/jni/jni_client.h (right):

https://codereview.chromium.org/2007403002/diff/20001/remoting/client/jni/jni...
remoting/client/jni/jni_client.h:89: // JNI Interface.
On 2016/05/25 20:51:50, Lambros wrote:
> Maybe remove this comment, or say instead "The following methods are exposed
to
> Java via JNI."

Done.

https://codereview.chromium.org/2007403002/diff/20001/remoting/client/jni/jni...
remoting/client/jni/jni_client.h:164: base::WeakPtr<JniClient> GetWeakPtr();
On 2016/05/25 20:51:50, Lambros wrote:
> Optional, I don't mind either way:
> Instead of exposing a weak pointer in the public API, I think our usual
pattern
> is to have Foo() and DoFoo() methods, where Foo() can be called on any thread,
> and it trampolines to DoFoo() on the right thread. A lot of the time, Foo()
and
> DoFoo() are actually the same method.

Left a TODO comment

https://codereview.chromium.org/2007403002/diff/20001/remoting/client/jni/jni...
remoting/client/jni/jni_client.h:173: // Pointer to the Java client object.
On 2016/05/25 20:51:50, Lambros wrote:
> s/Pointer/Reference/

Done.

[Yuwei, 2016-05-25 21:20:06]

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

[commit-bot: I haz the power, 2016-05-25 21:21:09]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2007403002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2007403002/60001

[commit-bot: I haz the power, 2016-05-25 23:19:08]

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

[commit-bot: I haz the power, 2016-05-25 23:19:09]

Dry run: This issue passed the CQ dry run.

[commit-bot: I haz the power, 2016-05-25 23:19:32]

Dry run: None

[Yuwei, 2016-05-25 23:19:54]

The CQ bit was checked by yuweih@chromium.org

[Yuwei, 2016-05-25 23:19:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
lambroslambrou@chromium.org
Link to the patchset: https://codereview.chromium.org/2007403002/#ps60001
(title: "Reviewer's Feedback")

[commit-bot: I haz the power, 2016-05-25 23:20:42]

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

[commit-bot: I haz the power, 2016-05-25 23:27:51]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-05-25 23:30:16]

Description was changed from

==========
[Android Client] Move session-scoped native interface into JniClient

This finishes up the refactorization of the JNI interface. Scheduled tasks
inside a session will not be lingering after the session ends.

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

to

==========
[Android Client] Move session-scoped native interface into JniClient

This finishes up the refactorization of the JNI interface. Scheduled tasks
inside a session will not be lingering after the session ends.

BUG=526336

Committed: https://crrev.com/800991e54ced74fc425843070a1941827af5b787
Cr-Commit-Position: refs/heads/master@{#396036}
==========

[commit-bot: I haz the power, 2016-05-25 23:30:17]

Patchset 3 (id:??) landed as
https://crrev.com/800991e54ced74fc425843070a1941827af5b787
Cr-Commit-Position: refs/heads/master@{#396036}

[Yuwei, 2016-05-31 19:27:32]

A revert of this CL (patchset #3 id:60001) has been created in
https://codereview.chromium.org/2027783002/ by yuweih@chromium.org.

The reason for reverting is: Revert this patch so that a fix for random crashes
can land on M52 without conflicts..

[Yuwei, 2016-06-01 17:35:29]

Description was changed from

==========
[Android Client] Move session-scoped native interface into JniClient

This finishes up the refactorization of the JNI interface. Scheduled tasks
inside a session will not be lingering after the session ends.

BUG=526336

Committed: https://crrev.com/800991e54ced74fc425843070a1941827af5b787
Cr-Commit-Position: refs/heads/master@{#396036}
==========

to

==========
[Android Client] Move session-scoped native interface into JniClient

This finishes up the refactorization of the JNI interface. Scheduled tasks
inside a session will not be lingering after the session ends.

BUG=526336

Committed: https://crrev.com/800991e54ced74fc425843070a1941827af5b787
Cr-Commit-Position: refs/heads/master@{#396036}
==========

[Yuwei, 2016-06-01 18:01:10]

The CQ bit was checked by yuweih@chromium.org

[Yuwei, 2016-06-01 18:01:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
lambroslambrou@chromium.org
Link to the patchset: https://codereview.chromium.org/2007403002/#ps80001
(title: "Merged with latest changes")

[commit-bot: I haz the power, 2016-06-01 18:01:33]

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

[commit-bot: I haz the power, 2016-06-01 19:36:20]

Description was changed from

==========
[Android Client] Move session-scoped native interface into JniClient

This finishes up the refactorization of the JNI interface. Scheduled tasks
inside a session will not be lingering after the session ends.

BUG=526336

Committed: https://crrev.com/800991e54ced74fc425843070a1941827af5b787
Cr-Commit-Position: refs/heads/master@{#396036}
==========

to

==========
[Android Client] Move session-scoped native interface into JniClient

This finishes up the refactorization of the JNI interface. Scheduled tasks
inside a session will not be lingering after the session ends.

BUG=526336

Committed: https://crrev.com/800991e54ced74fc425843070a1941827af5b787
Cr-Commit-Position: refs/heads/master@{#396036}
==========

[commit-bot: I haz the power, 2016-06-01 19:36:21]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-06-01 19:41:30]

Description was changed from

==========
[Android Client] Move session-scoped native interface into JniClient

This finishes up the refactorization of the JNI interface. Scheduled tasks
inside a session will not be lingering after the session ends.

BUG=526336

Committed: https://crrev.com/800991e54ced74fc425843070a1941827af5b787
Cr-Commit-Position: refs/heads/master@{#396036}
==========

to

==========
[Android Client] Move session-scoped native interface into JniClient

This finishes up the refactorization of the JNI interface. Scheduled tasks
inside a session will not be lingering after the session ends.

BUG=526336

Committed: https://crrev.com/800991e54ced74fc425843070a1941827af5b787
Cr-Commit-Position: refs/heads/master@{#396036}

Committed: https://crrev.com/6a7a5a02baacc4b1ddaf58a22d03ffe723a263c7
Cr-Commit-Position: refs/heads/master@{#397209}
==========

[commit-bot: I haz the power, 2016-06-01 19:41:32]

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