Upstream: Extract "WebAPK service" implementation from Chrome APK

Upstream: Extract "WebAPK service" implementation from Chrome APK

Ideally as much WebAPK code as possible can be updated without a new WebAPK
being downloaded from the Chrome server. This CL makes the WebAPK extract
a dex with the "WebAPK service" implementation from the Chrome APK and use
the extracted dex via reflection.

BUG=609122
Committed: https://crrev.com/fdc0ccb04b6572f4b25087ec29874bdc25657fff
Cr-Commit-Position: refs/heads/master@{#396645}

[pkotwicz, 2016-05-25 01:51:21]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2016-05-25 01:56:04]

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

[pkotwicz, 2016-05-25 01:56:05]

Xi can you please take a look? This CL depends on
https://codereview.chromium.org/2006703004 which adds FileUtils#extractAsset()

[pkotwicz, 2016-05-25 01:56:20]

Description was changed from

==========
Extract "WebAPK service" implementation from Chrome APK

Ideally as much WebAPK code as possible can be updated without a new WebAPK
being downloaded from the Chrome server. This CL makes the WebAPK extract
a dex with the "WebAPK service" implementation from the Chrome APK and use
the extracted dex via reflection.

BUG=609122
==========

to

==========
Extract "WebAPK service" implementation from Chrome APK

Ideally as much WebAPK code as possible can be updated without a new WebAPK
being downloaded from the Chrome server. This CL makes the WebAPK extract
a dex with the "WebAPK service" implementation from the Chrome APK and use
the extracted dex via reflection.

BUG=609122
==========

[Xi Han, 2016-05-25 15:36:13]

https://codereview.chromium.org/2011613003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkVersionManager.java
(right):

https://codereview.chromium.org/2011613003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkVersionManager.java:43:
if (preferences.getBoolean(TRIED_EXTRACTING_DEX_PREF, false)) {
Please move the comments in line 38 and 39 right above this "if" check. Or maybe
to have a function needToUpdate() and change the check from: 
if (preferences.getBoolean(TRIED_EXTRACTING_DEX_PREF, false)) to
if (!needToUpdate())
and move the comment to needToUpdate().

It is because that the above check always leads to an early exist, a little
confusing without a good explanation.

[pkotwicz, 2016-05-25 17:16:10]

Patchset #2 (id:40001) has been deleted

[pkotwicz, 2016-05-25 17:16:42]

Xi can you please take another look?

https://codereview.chromium.org/2011613003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkVersionManager.java
(right):

https://codereview.chromium.org/2011613003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkVersionManager.java:43:
if (preferences.getBoolean(TRIED_EXTRACTING_DEX_PREF, false)) {
I removed the comment entirely for now :)

[Xi Han, 2016-05-25 17:24:05]

lgtm, and I would like to see the updates after adding the WebAPK versioning
part back:)

[pkotwicz, 2016-05-25 18:21:07]

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

[pkotwicz, 2016-05-25 18:21:08]

dfalcantara@ for changes to chrome/android/java

[gone, 2016-05-25 21:11:02]

lgtm % checking with wnwen about the Application context.  I don't understand
the ClassLoader stuff well, though, so I'll rely on hanxi@'s judgement.

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkVersionManager.java
(right):

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkVersionManager.java:26:
"web_apk_version_manager_tried_extracting_dex";
nit: Prepend the package here instead:

"org.chromium.chrome.browser.webapps.TRIED_EXTRACTING_DEX"

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkVersionManager.java:28:
private static final String TAG = "WebApkVersionManager";
TAG is only necessary if you have logs, which you don't use.

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkVersionManager.java:33:
* @param Context context
Fill in the @param description or remove it.  It's not helpful as is.

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkVersionManager.java:39:
PreferenceManager.getDefaultSharedPreferences(context.getApplicationContext());
Can you use ContextUtils.getApplicationContext() here instead?  Check with
wnwen@

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/webapk/l...
File
chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkUtils.java
(right):

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/webapk/l...
chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkUtils.java:18:
private static final String DEFAULT_CHROME_PACKAGE_NAME =
"com.google.android.apps.chrome";
nit: Kind of weird to hard-code this.  Is there anywhere else this can be
grabbed from?

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/webapk/l...
chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkUtils.java:34:
return context.getApplicationContext().createPackageContext(
ContextUtils.getApplicationContext()?

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/webapk/s...
File
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/WebApkServiceFactory.java
(right):

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/WebApkServiceFactory.java:23:
private static final String TAG = "cr.WebApkServiceFactory";
nit: cr. should be automatically added now.  Just make it WebApkServiceFactory.

[pkotwicz, 2016-05-26 01:09:11]

Patchset #3 (id:80001) has been deleted

[pkotwicz, 2016-05-26 01:11:40]

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

[pkotwicz, 2016-05-26 01:11:42]

wnwen@ can you please take a look at the use of ContextUtils in
WebApkVersionManager.java

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/webapk/l...
File
chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkUtils.java
(right):

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/webapk/l...
chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkUtils.java:18:
private static final String DEFAULT_CHROME_PACKAGE_NAME =
"com.google.android.apps.chrome";
Ok, I have removed the default.

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/webapk/l...
chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkUtils.java:34:
return context.getApplicationContext().createPackageContext(
Unfortunately the code in chrome/android/webapk cannot depend on //base (because
it is for an APK which is completely separate from Chrome and should be as small
as possible)

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/webapk/s...
File
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/WebApkServiceFactory.java
(right):

https://codereview.chromium.org/2011613003/diff/60001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/WebApkServiceFactory.java:23:
private static final String TAG = "cr.WebApkServiceFactory";
The WebAPK code cannot depend on //base This means that we have to use
android.util.Log, not org.chromium.base.Log

[Peter Wen, 2016-05-26 15:19:39]

lgtm, ContextUtils is already set up when DeferredHandler runs.

https://codereview.chromium.org/2011613003/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkVersionManager.java
(right):

https://codereview.chromium.org/2011613003/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkVersionManager.java:40:
SharedPreferences.Editor editor = preferences.edit();
It's often cleaner to do this in one line, but it's just personal preferences:

preferences.edit().putBoolean(...).apply();

[pkotwicz, 2016-05-26 18:21:54]

Description was changed from

==========
Extract "WebAPK service" implementation from Chrome APK

Ideally as much WebAPK code as possible can be updated without a new WebAPK
being downloaded from the Chrome server. This CL makes the WebAPK extract
a dex with the "WebAPK service" implementation from the Chrome APK and use
the extracted dex via reflection.

BUG=609122
==========

to

==========
Upstream: Extract "WebAPK service" implementation from Chrome APK

Ideally as much WebAPK code as possible can be updated without a new WebAPK
being downloaded from the Chrome server. This CL makes the WebAPK extract
a dex with the "WebAPK service" implementation from the Chrome APK and use
the extracted dex via reflection.

BUG=609122
==========

[pkotwicz, 2016-05-28 03:46:59]

Patchset #5 (id:140001) has been deleted

[pkotwicz, 2016-05-28 17:20:17]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2016-05-28 17:20:18]

The patchset sent to the CQ was uploaded after l-g-t-m from hanxi@chromium.org,
dfalcantara@chromium.org, wnwen@chromium.org
Link to the patchset: https://codereview.chromium.org/2011613003/#ps160001
(title: " ")

[commit-bot: I haz the power, 2016-05-28 17:20:29]

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

[commit-bot: I haz the power, 2016-05-28 18:35:30]

Description was changed from

==========
Upstream: Extract "WebAPK service" implementation from Chrome APK

Ideally as much WebAPK code as possible can be updated without a new WebAPK
being downloaded from the Chrome server. This CL makes the WebAPK extract
a dex with the "WebAPK service" implementation from the Chrome APK and use
the extracted dex via reflection.

BUG=609122
==========

to

==========
Upstream: Extract "WebAPK service" implementation from Chrome APK

Ideally as much WebAPK code as possible can be updated without a new WebAPK
being downloaded from the Chrome server. This CL makes the WebAPK extract
a dex with the "WebAPK service" implementation from the Chrome APK and use
the extracted dex via reflection.

BUG=609122
==========

[commit-bot: I haz the power, 2016-05-28 18:35:31]

Committed patchset #5 (id:160001)

[commit-bot: I haz the power, 2016-05-28 18:36:56]

Description was changed from

==========
Upstream: Extract "WebAPK service" implementation from Chrome APK

Ideally as much WebAPK code as possible can be updated without a new WebAPK
being downloaded from the Chrome server. This CL makes the WebAPK extract
a dex with the "WebAPK service" implementation from the Chrome APK and use
the extracted dex via reflection.

BUG=609122
==========

to

==========
Upstream: Extract "WebAPK service" implementation from Chrome APK

Ideally as much WebAPK code as possible can be updated without a new WebAPK
being downloaded from the Chrome server. This CL makes the WebAPK extract
a dex with the "WebAPK service" implementation from the Chrome APK and use
the extracted dex via reflection.

BUG=609122

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

[commit-bot: I haz the power, 2016-05-28 18:36:57]

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