Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(11)

Issue 1971773002: Upstream: Add WebAPK's client library. (Closed)

Created:
4 years, 7 months ago by Xi Han
Modified:
4 years, 7 months ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add WebAPK's client library. Introduce WebApkValidator that verifies whether a WebAPK is signed by WebAPK Minting Server, and whether a URL can be handled by a WebAPK. - It is in WebAPK's client library, which provides APIs for WebAPK host to communicate with WebAPKs. - ChromeWebApkHost is introduced to initialize the public key for WebAPK's signature verification. BUG=609122 Committed: https://crrev.com/5cb2361c8307d9df549c357b56efe44c256580f3 Cr-Commit-Position: refs/heads/master@{#393674}

Patch Set 1 : #

Total comments: 10

Patch Set 2 : Remove runtime_library. #

Patch Set 3 : Add WebApkValidatorTest. #

Total comments: 22

Patch Set 4 : yfriedman@'s comments. #

Total comments: 2

Patch Set 5 : Revert changes in chromium.linux.json. #

Patch Set 6 : Add a test. #

Patch Set 7 : Add DEPS. #

Messages

Total messages: 47 (25 generated)
Xi Han
Hi Peter, can you please take a look? Thanks!
4 years, 7 months ago (2016-05-11 18:12:52 UTC) #7
Xi Han
4 years, 7 months ago (2016-05-11 18:43:04 UTC) #9
pkotwicz
LGTM for libs/client Can you please split the runtime_library part into a separate CL. When ...
4 years, 7 months ago (2016-05-11 20:52:18 UTC) #11
Xi Han
Hi Robert, could you please review the validation of WebAPKs in WebApkValidator and ChromeWebApkHost? Thanks! ...
4 years, 7 months ago (2016-05-11 21:16:26 UTC) #15
Robert Sesek
Code lg, but I'd like to see unit tests for WebApkValidator as well, since it's ...
4 years, 7 months ago (2016-05-12 17:22:51 UTC) #18
Xi Han
Hi Robert, I added a junit test, please take another look, thanks!
4 years, 7 months ago (2016-05-12 21:50:47 UTC) #19
Yaron
https://codereview.chromium.org/1971773002/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeWebApkHost.java File chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeWebApkHost.java (right): https://codereview.chromium.org/1971773002/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeWebApkHost.java#newcode65 chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeWebApkHost.java:65: WebApkValidator.initWithBrowserHostSignature(EXPECTED_SIGNATURE); can we add the command line flag for ...
4 years, 7 months ago (2016-05-12 22:11:40 UTC) #21
Xi Han
Hi Yaron: PTAL, thanks! https://codereview.chromium.org/1971773002/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeWebApkHost.java File chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeWebApkHost.java (right): https://codereview.chromium.org/1971773002/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeWebApkHost.java#newcode65 chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeWebApkHost.java:65: WebApkValidator.initWithBrowserHostSignature(EXPECTED_SIGNATURE); On 2016/05/12 22:11:39, Yaron ...
4 years, 7 months ago (2016-05-13 15:42:55 UTC) #23
Xi Han
https://codereview.chromium.org/1971773002/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeWebApkHost.java File chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeWebApkHost.java (right): https://codereview.chromium.org/1971773002/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeWebApkHost.java#newcode65 chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeWebApkHost.java:65: WebApkValidator.initWithBrowserHostSignature(EXPECTED_SIGNATURE); On 2016/05/13 15:42:54, Xi Han wrote: > On ...
4 years, 7 months ago (2016-05-13 15:44:54 UTC) #24
Yaron
lgtm but you'll need a new reviewer for the bot change https://codereview.chromium.org/1971773002/diff/160001/chrome/android/webapk/libs/client/junit/src/org/chromium/webapk/lib/client/WebApkValidatorTest.java File chrome/android/webapk/libs/client/junit/src/org/chromium/webapk/lib/client/WebApkValidatorTest.java (right): ...
4 years, 7 months ago (2016-05-13 15:50:13 UTC) #25
pkotwicz
I suggest doing the bot changes as a separate CL
4 years, 7 months ago (2016-05-13 15:52:09 UTC) #26
Yaron
fine with me On Fri, May 13, 2016 at 11:52 AM <pkotwicz@chromium.org> wrote: > I ...
4 years, 7 months ago (2016-05-13 15:52:51 UTC) #27
Xi Han
Revert the changes in chromium.linux.json, and will submit a separate CL to add tests in ...
4 years, 7 months ago (2016-05-13 15:57:47 UTC) #28
Robert Sesek
LGTM One more test to consider: testing if a package doesn't start with WEBAPK_PACKAGE_PREFIX
4 years, 7 months ago (2016-05-13 19:01:45 UTC) #30
Xi Han
Good idea, added another test.
4 years, 7 months ago (2016-05-13 19:14:22 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1971773002/220001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1971773002/220001
4 years, 7 months ago (2016-05-13 19:18:07 UTC) #34
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/228682)
4 years, 7 months ago (2016-05-13 20:49:47 UTC) #36
Xi Han
Hi Dirk, could you please take a look */DEPS? Thanks!
4 years, 7 months ago (2016-05-13 21:24:36 UTC) #39
Dirk Pranke
lgtm.
4 years, 7 months ago (2016-05-13 21:33:55 UTC) #40
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1971773002/260001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1971773002/260001
4 years, 7 months ago (2016-05-13 21:34:49 UTC) #43
commit-bot: I haz the power
Committed patchset #7 (id:260001)
4 years, 7 months ago (2016-05-13 23:05:00 UTC) #45
commit-bot: I haz the power
4 years, 7 months ago (2016-05-13 23:06:42 UTC) #47
Message was sent while issue was closed.
Patchset 7 (id:??) landed as
https://crrev.com/5cb2361c8307d9df549c357b56efe44c256580f3
Cr-Commit-Position: refs/heads/master@{#393674}

Powered by Google App Engine
This is Rietveld 408576698