[Remoting Android] Fix crash caused by fetchAuthToken()

[Remoting Android] Fix crash caused by fetchAuthToken()

Previously auth token was only fetched after the account switcher set the
account. A recent refactoring makes the app attempt to fetch the auth token
even before the account is set, so the fetchAuthToken() function just throws
an exception and crashes the app.

Since we are pretty sure that the Java code will soon feed the logger with the
auth code once the account is set, it's safe to fix this by loosening the check
to just a warning log.

Alternatively we can remove the immediate RequestAuthTokenForLogger() call from
ChromotingClientRuntime and make the logger request the auth token before it
tries to send out the logs. It depends on how the iOS client wants to provide
the auth token.

BUG=703381
Review-Url: https://codereview.chromium.org/2761983002
Cr-Commit-Position: refs/heads/master@{#458515}
Committed: https://chromium.googlesource.com/chromium/src/+/ced631a3b4de3fd68d98ecc1620d76b47a5ea9b2

[Yuwei, 2017-03-20 22:38:27]

Description was changed from

==========
[Remoting Android] Fix crash caused by fetchAuthToken()

Previously auth token was only fetched after the account switcher set the
account. A recent refactoring makes the app attempt to fetch the auth token
even before the account is set, so the fetchAuthToken() function just throws
an exception and crashes the app.

Since we are pretty sure that the Java code will soon feed the logger with the
auth code once the account is set, it's safe to fix this by loosening the check
to just a warning log.

Alternatively we can remove the immediate RequestAuthTokenForLogger() call from
ChromotingClientRuntime and make the logger to request the auth token before it
tries to send out the logs. It depends on how the iOS client wants to provide
the auth token.

BUG=703381
==========

to

==========
[Remoting Android] Fix crash caused by fetchAuthToken()

Previously auth token was only fetched after the account switcher set the
account. A recent refactoring makes the app attempt to fetch the auth token
even before the account is set, so the fetchAuthToken() function just throws
an exception and crashes the app.

Since we are pretty sure that the Java code will soon feed the logger with the
auth code once the account is set, it's safe to fix this by loosening the check
to just a warning log.

Alternatively we can remove the immediate RequestAuthTokenForLogger() call from
ChromotingClientRuntime and make the logger request the auth token before it
tries to send out the logs. It depends on how the iOS client wants to provide
the auth token.

BUG=703381
==========

[Yuwei, 2017-03-20 22:40:06]

yuweih@chromium.org changed reviewers:
+ nicholss@chromium.org, sergeyu@chromium.org

[Yuwei, 2017-03-20 22:40:07]

PTAL. Thanks!

Please also see the alternative solution mentioned in the description.

[nicholss, 2017-03-21 14:37:33]

On 2017/03/20 22:40:07, Yuwei wrote:
> PTAL. Thanks!
> 
> Please also see the alternative solution mentioned in the description.

RequestAuthTokenForLogger does not have to be serviced if there is no auth token
to be had. it will continue to request for a token each time it attempts to send
logs if I understand it correctly. you can just drop the requests as they come
until you have the ability to feed it a valid token.

[Sergey Ulanov, 2017-03-21 18:39:37]

lgtm

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

https://codereview.chromium.org/2761983002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java:62:
Log.w(TAG, "Account is not set before fetching the auth token.");
Maybe add a comment here to explain why we can ignore this request.

[Yuwei, 2017-03-21 18:52:44]

On 2017/03/21 14:37:33, nicholss wrote:
> On 2017/03/20 22:40:07, Yuwei wrote:
> > PTAL. Thanks!
> > 
> > Please also see the alternative solution mentioned in the description.
> 
> RequestAuthTokenForLogger does not have to be serviced if there is no auth
token
> to be had. it will continue to request for a token each time it attempts to
send
> logs if I understand it correctly. you can just drop the requests as they come
> until you have the ability to feed it a valid token.

> it will continue to request for a token each time it attempts to send logs if
I understand it correctly. 
Not each time but once when the logger fails to send a log due to 401 error. It
should be changed to fetch the token immediately before sending the first log if
the token has not been set.

> you can just drop the requests as they come until you have the ability to feed
it a valid token.
This might be a good idea if iOS also doesn't have the user account info
available right after the app is launched.

I think this fix is still useful in the sense that logs generated at launch time
can still be queued up and won't crash the app. I will send follow-up CL for the
logger.

[Yuwei, 2017-03-21 19:02:24]

Thanks!

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

https://codereview.chromium.org/2761983002/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java:62:
Log.w(TAG, "Account is not set before fetching the auth token.");
On 2017/03/21 18:39:37, Sergey Ulanov wrote:
> Maybe add a comment here to explain why we can ignore this request.

Done.

[Yuwei, 2017-03-21 19:02:35]

The CQ bit was checked by yuweih@chromium.org

[Yuwei, 2017-03-21 19:02:36]

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/2761983002/#ps20001
(title: "Reviewer's Feedback / Add Comments")

[commit-bot: I haz the power, 2017-03-21 19:03:42]

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, 2017-03-21 19:41:51]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490122955868270,
"parent_rev": "c57fb88d3baa0ca58f4b061c2c085054794e5a14", "commit_rev":
"ced631a3b4de3fd68d98ecc1620d76b47a5ea9b2"}

[commit-bot: I haz the power, 2017-03-21 19:42:33]

Description was changed from

==========
[Remoting Android] Fix crash caused by fetchAuthToken()

Previously auth token was only fetched after the account switcher set the
account. A recent refactoring makes the app attempt to fetch the auth token
even before the account is set, so the fetchAuthToken() function just throws
an exception and crashes the app.

Since we are pretty sure that the Java code will soon feed the logger with the
auth code once the account is set, it's safe to fix this by loosening the check
to just a warning log.

Alternatively we can remove the immediate RequestAuthTokenForLogger() call from
ChromotingClientRuntime and make the logger request the auth token before it
tries to send out the logs. It depends on how the iOS client wants to provide
the auth token.

BUG=703381
==========

to

==========
[Remoting Android] Fix crash caused by fetchAuthToken()

Previously auth token was only fetched after the account switcher set the
account. A recent refactoring makes the app attempt to fetch the auth token
even before the account is set, so the fetchAuthToken() function just throws
an exception and crashes the app.

Since we are pretty sure that the Java code will soon feed the logger with the
auth code once the account is set, it's safe to fix this by loosening the check
to just a warning log.

Alternatively we can remove the immediate RequestAuthTokenForLogger() call from
ChromotingClientRuntime and make the logger request the auth token before it
tries to send out the logs. It depends on how the iOS client wants to provide
the auth token.

BUG=703381

Review-Url: https://codereview.chromium.org/2761983002
Cr-Commit-Position: refs/heads/master@{#458515}
Committed:
https://chromium.googlesource.com/chromium/src/+/ced631a3b4de3fd68d98ecc1620d...
==========

[commit-bot: I haz the power, 2017-03-21 19:42:34]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/ced631a3b4de3fd68d98ecc1620d...