Make HostBrowserClassLoader#getClassLoaderInstance() work if Chrome gets updated

Make HostBrowserClassLoader#getClassLoaderInstance() work if Chrome gets updated

WebAPK process is not guaranteed to terminate when Chrome gets updated. The
WebAPK code assumes that it is always using the latest version of runtime
library. In order to guarantee this, the ClassLoader must be re-created if Chrome
is updated but the WebAPK is still running.

BUG=627950
TEST=HostBrowserLauncherTest.*
TBR=hanxi (TBR for DEP on testing/ by JUnit test)

Committed: https://crrev.com/b475d23eae38217dc460763f670e0fa150fea36e
Cr-Commit-Position: refs/heads/master@{#408751}

[pkotwicz, 2016-07-25 20:42:17]

pkotwicz@chromium.org changed reviewers:
+ hanxi@chromium.org

[pkotwicz, 2016-07-25 20:42:17]

Xi can you please take a look?

[pkotwicz, 2016-07-25 20:42:44]

Description was changed from

==========
Make HostBrowserClassLoader#getClassLoaderInstance() work if Chrome gets updated

WebAPK process is not guaranteed to terminate when Chrome gets updated. The
WebAPK code assumes that it is always using the latest version of runtime
library. In order to guarantee this, the ClassLoader must be recreated if Chrome
is updated but the WebAPK is still running.

BUG=627950
TEST=HostBrowserLauncherTest.*
==========

to

==========
Make HostBrowserClassLoader#getClassLoaderInstance() work if Chrome gets updated

WebAPK process is not guaranteed to terminate when Chrome gets updated. The
WebAPK code assumes that it is always using the latest version of runtime
library. In order to guarantee this, the ClassLoader must be re-created if
Chrome
is updated but the WebAPK is still running.

BUG=627950
TEST=HostBrowserLauncherTest.*
==========

[pkotwicz, 2016-07-25 20:45:29]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2016-07-25 20:49:40]

I am unsure how much of an issue the WebAPK still running but Chrome getting
updated is. Do we assume it won't happen?

Currently, it looks like the WebAPk process never ends

[pkotwicz, 2016-07-25 20:59:42]

I am pretty sure that we need to handle the WebAPK still running but Chrome
getting updated because if WebAPK is displaying a notification the notification
keeps the WebAPK running.

[Xi Han, 2016-07-27 14:56:30]

Mostly for comments.

https://codereview.chromium.org/2179163002/diff/20001/chrome/android/webapk/s...
File
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java
(right):

https://codereview.chromium.org/2179163002/diff/20001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java:41:
* ClassLoader for WebAPK dex. Static so that the same ClassLoader is used for
app's lifetime.
Please update this description by appending: "It will be re-created if the
WebAPK's host browser updated."

https://codereview.chromium.org/2179163002/diff/20001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java:101:
// reused in this scenario.
How about: "WebAPK may still be running when its host browser gets updated,
e.g., version code increases. Prevent ClassLoader from getting
reused in this scenario."

https://codereview.chromium.org/2179163002/diff/20001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java:106:
return remoteVersionCode == cachedRemoteVersionCode;
Is it possible that both |remoteVersionCode| and |cachedRemoteVersionCode| are
-1? I understand when this function is called in line 58, it guarantees that the
remoteContext isn't null. Since this is public function, we cannot always make
an assumption when it is called. For the case that |remoteContext| is null, will
it through an exception and crash the WebAPK, so the case of both values are -1
won't occur?

[pkotwicz, 2016-07-28 18:48:44]

Xi, can you please take another look?

https://codereview.chromium.org/2179163002/diff/20001/chrome/android/webapk/s...
File
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java
(right):

https://codereview.chromium.org/2179163002/diff/20001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java:101:
// reused in this scenario.
Changed "Chrome" to "host browser"

Skipped "e.g. version code" part because how we detect that the "host browser is
upgraded" is an implementation detail

https://codereview.chromium.org/2179163002/diff/20001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java:106:
return remoteVersionCode == cachedRemoteVersionCode;
I don't care about the return value when both |remoteVersionCode| and
|cachedRemoteVersionCode| are -1. I can't think of a scenario where
|sClassLoader| != null and both |remoteVersionCode| and
|cachedRemoteVersionCode| are -1.

[Xi Han, 2016-07-28 21:33:17]

lgtm

https://codereview.chromium.org/2179163002/diff/20001/chrome/android/webapk/s...
File
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java
(right):

https://codereview.chromium.org/2179163002/diff/20001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java:106:
return remoteVersionCode == cachedRemoteVersionCode;
On 2016/07/28 18:48:44, pkotwicz wrote:
> I don't care about the return value when both |remoteVersionCode| and
> |cachedRemoteVersionCode| are -1. I can't think of a scenario where
> |sClassLoader| != null and both |remoteVersionCode| and
> |cachedRemoteVersionCode| are -1.

In the way that how this function is expected to use, it isn't possible that
both values are -1. I am ok with this.

[pkotwicz, 2016-07-29 17:36:50]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2016-07-29 17:37:01]

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

[pkotwicz, 2016-07-29 17:37:54]

The CQ bit was unchecked by pkotwicz@chromium.org

[pkotwicz, 2016-07-29 17:39:14]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2016-07-29 17:39:34]

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-07-29 18:40:58]

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

[commit-bot: I haz the power, 2016-07-29 18:40:59]

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_...)

[pkotwicz, 2016-07-29 20:18:32]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2016-07-29 20:18:33]

The patchset sent to the CQ was uploaded after l-g-t-m from hanxi@chromium.org
Link to the patchset: https://codereview.chromium.org/2179163002/#ps60001
(title: "Merge branch 'master' into webapk_chrome_updated")

[commit-bot: I haz the power, 2016-07-29 20:19:02]

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-07-29 20:25:47]

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

[commit-bot: I haz the power, 2016-07-29 20:25:48]

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

[nabila mehjabin, 2016-07-29 20:34:19]

On 2016/07/29 20:25:48, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

Restart for http://www.apneserial.com service DNS address could on network

[nabila mehjabin, 2016-07-29 20:36:43]

On 2016/07/28 18:48:44, pkotwicz wrote:
> Xi, can you please take another look?
> 
>
https://codereview.chromium.org/2179163002/diff/20001/chrome/android/webapk/s...
> File
>
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java
> (right):
> 
>
https://codereview.chromium.org/2179163002/diff/20001/chrome/android/webapk/s...
>
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java:101:
> // reused in this scenario.
> Changed "Chrome" to "host browser"
> 
> Skipped "e.g. version code" part because how we detect that the "host browser
is
> upgraded" is an implementation detail
> 
>
https://codereview.chromium.org/2179163002/diff/20001/chrome/android/webapk/s...
>
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java:106:
> return remoteVersionCode == cachedRemoteVersionCode;
> I don't care about the return value when both |remoteVersionCode| and
> |cachedRemoteVersionCode| are -1. I can't think of a scenario where
> |sClassLoader| != null and both |remoteVersionCode| and
> |cachedRemoteVersionCode| are -1.

Restart for https://www.apneserial.com service DNS address could on network 
service DNS

[pkotwicz, 2016-07-29 20:50:24]

Description was changed from

==========
Make HostBrowserClassLoader#getClassLoaderInstance() work if Chrome gets updated

WebAPK process is not guaranteed to terminate when Chrome gets updated. The
WebAPK code assumes that it is always using the latest version of runtime
library. In order to guarantee this, the ClassLoader must be re-created if
Chrome
is updated but the WebAPK is still running.

BUG=627950
TEST=HostBrowserLauncherTest.*
==========

to

==========
Make HostBrowserClassLoader#getClassLoaderInstance() work if Chrome gets updated

WebAPK process is not guaranteed to terminate when Chrome gets updated. The
WebAPK code assumes that it is always using the latest version of runtime
library. In order to guarantee this, the ClassLoader must be re-created if
Chrome
is updated but the WebAPK is still running.

BUG=627950
TEST=HostBrowserLauncherTest.*
TBR=hanxi (TBR for DEP on testing/ by JUnit test)
==========

[pkotwicz, 2016-07-29 20:50:28]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2016-07-29 20:51:08]

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-07-29 21:14:01]

Description was changed from

==========
Make HostBrowserClassLoader#getClassLoaderInstance() work if Chrome gets updated

WebAPK process is not guaranteed to terminate when Chrome gets updated. The
WebAPK code assumes that it is always using the latest version of runtime
library. In order to guarantee this, the ClassLoader must be re-created if
Chrome
is updated but the WebAPK is still running.

BUG=627950
TEST=HostBrowserLauncherTest.*
TBR=hanxi (TBR for DEP on testing/ by JUnit test)
==========

to

==========
Make HostBrowserClassLoader#getClassLoaderInstance() work if Chrome gets updated

WebAPK process is not guaranteed to terminate when Chrome gets updated. The
WebAPK code assumes that it is always using the latest version of runtime
library. In order to guarantee this, the ClassLoader must be re-created if
Chrome
is updated but the WebAPK is still running.

BUG=627950
TEST=HostBrowserLauncherTest.*
TBR=hanxi (TBR for DEP on testing/ by JUnit test)
==========

[commit-bot: I haz the power, 2016-07-29 21:14:02]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-07-29 21:15:42]

Description was changed from

==========
Make HostBrowserClassLoader#getClassLoaderInstance() work if Chrome gets updated

WebAPK process is not guaranteed to terminate when Chrome gets updated. The
WebAPK code assumes that it is always using the latest version of runtime
library. In order to guarantee this, the ClassLoader must be re-created if
Chrome
is updated but the WebAPK is still running.

BUG=627950
TEST=HostBrowserLauncherTest.*
TBR=hanxi (TBR for DEP on testing/ by JUnit test)
==========

to

==========
Make HostBrowserClassLoader#getClassLoaderInstance() work if Chrome gets updated

WebAPK process is not guaranteed to terminate when Chrome gets updated. The
WebAPK code assumes that it is always using the latest version of runtime
library. In order to guarantee this, the ClassLoader must be re-created if
Chrome
is updated but the WebAPK is still running.

BUG=627950
TEST=HostBrowserLauncherTest.*
TBR=hanxi (TBR for DEP on testing/ by JUnit test)

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

[commit-bot: I haz the power, 2016-07-29 21:15:43]

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