Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(19)

Issue 2629573004: Add a chrome://webapks page. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 months, 1 week ago by gonzalon
Modified:
8 months, 2 weeks ago
CC:
chromium-reviews, dominickn+watch_chromium.org, pkotwicz+watch_chromium.org, zpeng+watch_chromium.org, oshima+watch_chromium.org, agrieve+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add a chrome://webapks page. Adds an about:webapks page with information about all installed Web APKs on the device Design doc: go/about-webapks BUG=624111 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2629573004 Cr-Commit-Position: refs/heads/master@{#448747} Committed: https://chromium.googlesource.com/chromium/src/+/87192d7728d4f0829ba7eba2c49bb61810b0ea1a

Patch Set 1 #

Patch Set 2 : Adds an about:webapks page with information about all installed Web APKs on the device #

Total comments: 74

Patch Set 3 : Fixes code review style issues #

Patch Set 4 : Adds comment to explain the reason of an empty param #

Total comments: 6

Patch Set 5 : Some style fixes #

Patch Set 6 : Changes the arguments of the WebAPKs list callback to return a struct #

Patch Set 7 : Makes the listWebApks method receive a callback on the Java side #

Total comments: 48

Patch Set 8 : Fixes some more style issues #

Total comments: 13

Patch Set 9 : Removes strings from i18n file #

Total comments: 48

Patch Set 10 : Adds an about:webapks page with information about all installed Web APKs on the device #

Total comments: 14

Patch Set 11 : Adds an about:webapks page with information about all installed Web APKs on the device #

Total comments: 33

Patch Set 12 : Add a chrome://webapks page. #

Total comments: 2

Patch Set 13 : Add a chrome://webapks page. #

Total comments: 30

Patch Set 14 : Add a chrome://webapks page. #

Patch Set 15 : Add a chrome://webapks page. #

Total comments: 4

Patch Set 16 : Add a chrome://webapks page. #

Total comments: 6

Patch Set 17 : Add a chrome://webapks page. #

Patch Set 18 : Add a chrome://webapks page. #

Total comments: 10

Patch Set 19 : Add a chrome://webapks page. #

Patch Set 20 : Add a chrome://webapks page. #

Total comments: 2

Patch Set 21 : Add a chrome://webapks page. #

Total comments: 11

Patch Set 22 : Add a chrome://webapks page. #

Patch Set 23 : Add a chrome://webapks page. #

Patch Set 24 : Add a chrome://webapks page. #

Patch Set 25 : Add a chrome://webapks page. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+547 lines, -2 lines) Patch
M chrome/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +1 line, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 5 chunks +47 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +11 lines, -0 lines 0 comments Download
M chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java View 1 2 3 4 5 6 7 8 9 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/android/shortcut_helper.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 3 chunks +11 lines, -0 lines 0 comments Download
M chrome/browser/android/shortcut_helper.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 3 chunks +46 lines, -0 lines 0 comments Download
A chrome/browser/android/webapk/webapk_info.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +44 lines, -0 lines 0 comments Download
A chrome/browser/android/webapk/webapk_info.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +16 lines, -0 lines 0 comments Download
M chrome/browser/resources/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 22 1 chunk +11 lines, -0 lines 0 comments Download
A chrome/browser/resources/webapks/about_webapks.css View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +48 lines, -0 lines 0 comments Download
A chrome/browser/resources/webapks/about_webapks.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +32 lines, -0 lines 0 comments Download
A chrome/browser/resources/webapks/about_webapks.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +71 lines, -0 lines 0 comments Download
A chrome/browser/resources/webapks/compiled_resources2.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +15 lines, -0 lines 0 comments Download
A chrome/browser/resources/webapks_ui_resources.grd View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +16 lines, -0 lines 0 comments Download
M chrome/browser/ui/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/DEPS View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +3 lines, -0 lines 0 comments Download
A chrome/browser/ui/webui/webapks_handler.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +43 lines, -0 lines 0 comments Download
A chrome/browser/ui/webui/webapks_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +44 lines, -0 lines 0 comments Download
A chrome/browser/ui/webui/webapks_ui.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +21 lines, -0 lines 0 comments Download
A chrome/browser/ui/webui/webapks_ui.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +43 lines, -0 lines 0 comments Download
M chrome/chrome_paks.gni View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/common/url_constants.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/common/url_constants.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 3 chunks +3 lines, -0 lines 0 comments Download
M third_party/closure_compiler/compiled_resources2.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +1 line, -0 lines 0 comments Download
M tools/gritsettings/resource_ids View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +3 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 128 (56 generated)
gonzalon
Hi! Would you mind taking a look at this CL before I send it to ...
9 months, 1 week ago (2017-01-12 23:20:03 UTC) #9
hartmanng
https://codereview.chromium.org/2629573004/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebapkInfoCallback.java File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebapkInfoCallback.java (right): https://codereview.chromium.org/2629573004/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebapkInfoCallback.java#newcode1 chrome/android/java/src/org/chromium/chrome/browser/webapps/WebapkInfoCallback.java:1: // Copyright 2016 The Chromium Authors. All rights reserved. ...
9 months, 1 week ago (2017-01-13 15:40:22 UTC) #12
Xi Han
Overall looks good on the first round. Thank you gonzalon@ for taking care of this! ...
9 months, 1 week ago (2017-01-13 15:50:16 UTC) #13
pkotwicz
Still doing the code review. Some initial comments https://codereview.chromium.org/2629573004/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java (right): https://codereview.chromium.org/2629573004/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java#newcode184 chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java:184: if ...
9 months, 1 week ago (2017-01-13 16:00:54 UTC) #14
pkotwicz
https://codereview.chromium.org/2629573004/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java (right): https://codereview.chromium.org/2629573004/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java#newcode189 chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java:189: WebApkInfo webApkInfo = WebApkInfo.create(intent); I had code elsewhere which ...
9 months, 1 week ago (2017-01-13 16:23:19 UTC) #15
pkotwicz
Still reviewing but need to go for lunch https://codereview.chromium.org/2629573004/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebapkInfoCallback.java File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebapkInfoCallback.java (right): https://codereview.chromium.org/2629573004/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebapkInfoCallback.java#newcode29 chrome/android/java/src/org/chromium/chrome/browser/webapps/WebapkInfoCallback.java:29: public ...
9 months, 1 week ago (2017-01-13 17:00:59 UTC) #16
Xi Han
https://codereview.chromium.org/2629573004/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java (right): https://codereview.chromium.org/2629573004/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java#newcode189 chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java:189: WebApkInfo webApkInfo = WebApkInfo.create(intent); On 2017/01/13 16:23:19, pkotwicz wrote: ...
9 months, 1 week ago (2017-01-13 17:52:43 UTC) #17
gonzalon
A lot of lines were added to the BUILD file, but I didn't add them, ...
9 months, 1 week ago (2017-01-13 20:24:40 UTC) #20
Xi Han
Which BUILD file? They all look good to me. https://codereview.chromium.org/2629573004/diff/60001/chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java File chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java (right): https://codereview.chromium.org/2629573004/diff/60001/chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java#newcode16 chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java:16: ...
9 months, 1 week ago (2017-01-16 16:56:19 UTC) #21
gonzalon
Fixed the corrections, thanks a lot for the review! https://codereview.chromium.org/2629573004/diff/60001/chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java File chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java (right): https://codereview.chromium.org/2629573004/diff/60001/chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java#newcode16 chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java:16: ...
9 months, 1 week ago (2017-01-16 19:50:44 UTC) #22
pkotwicz
https://codereview.chromium.org/2629573004/diff/20001/chrome/browser/android/webapps/webapp_registry.cc File chrome/browser/android/webapps/webapp_registry.cc (right): https://codereview.chromium.org/2629573004/diff/20001/chrome/browser/android/webapps/webapp_registry.cc#newcode34 chrome/browser/android/webapps/webapp_registry.cc:34: callback) { I recommend doing this by making WebappRegistry::ListWebApks() ...
9 months, 1 week ago (2017-01-17 03:20:16 UTC) #23
pkotwicz
W.r.t to the BUILD.gn files, when you diff the BUILD.gn file of one patch set ...
9 months, 1 week ago (2017-01-17 03:40:13 UTC) #24
gonzalon
On 2017/01/17 03:40:13, pkotwicz wrote: > W.r.t to the BUILD.gn files, when you diff the ...
9 months, 1 week ago (2017-01-17 16:45:01 UTC) #25
gonzalon
https://codereview.chromium.org/2629573004/diff/20001/chrome/browser/android/webapps/webapp_registry.cc File chrome/browser/android/webapps/webapp_registry.cc (right): https://codereview.chromium.org/2629573004/diff/20001/chrome/browser/android/webapps/webapp_registry.cc#newcode34 chrome/browser/android/webapps/webapp_registry.cc:34: callback) { On 2017/01/17 03:20:16, pkotwicz wrote: > I ...
9 months, 1 week ago (2017-01-17 16:45:18 UTC) #26
pkotwicz
As discussed online I think that the solution that I suggested using multiple vectors is ...
9 months, 1 week ago (2017-01-17 19:47:20 UTC) #27
pkotwicz
As discussed online I think that the solution that I suggested using multiple vectors is ...
9 months, 1 week ago (2017-01-17 19:47:22 UTC) #28
pkotwicz
As discussed online I think that the solution that I suggested using multiple vectors is ...
9 months, 1 week ago (2017-01-17 19:47:23 UTC) #29
gonzalon
Implemented the changes we discussed offline.
9 months ago (2017-01-17 21:59:54 UTC) #30
pkotwicz
Looks really good. Once you address my comments I need to double check your comments ...
9 months ago (2017-01-18 02:08:21 UTC) #31
gonzalon
Thanks for you time reviewing! https://codereview.chromium.org/2629573004/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java File chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java (right): https://codereview.chromium.org/2629573004/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java#newcode663 chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java:663: List<String> shortNames = new ...
9 months ago (2017-01-18 15:29:32 UTC) #32
pkotwicz
Looking at your CL now. Sorry about the delay
9 months ago (2017-01-19 16:09:08 UTC) #33
gonzalon
@pkotwicz Removed the strings file as we talked offline.
9 months ago (2017-01-19 16:41:22 UTC) #34
pkotwicz
Mostly comment nits. There are a few comments from the last round of review that ...
9 months ago (2017-01-19 19:37:18 UTC) #36
agrieve
https://codereview.chromium.org/2629573004/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java File chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java (right): https://codereview.chromium.org/2629573004/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java#newcode688 chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java:688: private static int[] integerListToIntArray(@NonNull List<Integer> list) { On 2017/01/18 ...
9 months ago (2017-01-19 19:58:23 UTC) #37
gonzalon
I think I addressed all of your comments. I'm not very familiar with the chromium ...
9 months ago (2017-01-19 23:50:17 UTC) #38
pkotwicz
dbeam@ for code review of HTML/JS/CSS and chrome/browser/ui/webui dominickn@ for webapp and WebAPK code The ...
9 months ago (2017-01-20 16:57:17 UTC) #40
gonzalon
https://codereview.chromium.org/2629573004/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java (right): https://codereview.chromium.org/2629573004/diff/120001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java#newcode62 chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java:62: * manifest. On 2017/01/18 02:08:20, pkotwicz wrote: > Nit: ...
9 months ago (2017-01-20 19:27:01 UTC) #42
pkotwicz
https://codereview.chromium.org/2629573004/diff/200001/chrome/browser/android/shortcut_helper.h File chrome/browser/android/shortcut_helper.h (right): https://codereview.chromium.org/2629573004/diff/200001/chrome/browser/android/shortcut_helper.h#newcode29 chrome/browser/android/shortcut_helper.h:29: typedef base::Callback<void(std::vector<const WebApkInfo>&)> This should be: typedef base::Callback<void(const std::vector<WebApkInfo>&)> ...
9 months ago (2017-01-20 20:33:33 UTC) #43
dominickn
Please wrap the description to 80 characters, and change the title to be: Add a ...
9 months ago (2017-01-23 00:48:07 UTC) #44
pkotwicz
https://codereview.chromium.org/2629573004/diff/200001/chrome/browser/android/webapk/webapk_info.h File chrome/browser/android/webapk/webapk_info.h (right): https://codereview.chromium.org/2629573004/diff/200001/chrome/browser/android/webapk/webapk_info.h#newcode28 chrome/browser/android/webapk/webapk_info.h:28: const int version_code; I know that it is normal ...
9 months ago (2017-01-23 15:44:51 UTC) #46
gonzalon
I have a couple questions with two of the comments, thanks for your time reviewing! ...
9 months ago (2017-01-23 16:58:51 UTC) #48
dominickn
Please make "Add a chrome://webapks page." as the first line of the description (with a ...
9 months ago (2017-01-24 02:55:22 UTC) #49
dominickn
Had a really bad time typing all that: the WebApkInfo constructor call should be: webapk_list.push_back(WebApkInfo(std::move(short_names[i]), ...
9 months ago (2017-01-24 06:13:54 UTC) #50
gonzalon
Thanks a lot for the detailed explanation. I see I still have a lot to ...
9 months ago (2017-01-24 15:24:12 UTC) #52
Dan Beam (no longer on Chrome)
i'd prefer if we didn't use Unsafe() methods https://codereview.chromium.org/2629573004/diff/240001/chrome/browser/ui/webui/webapks_handler.cc File chrome/browser/ui/webui/webapks_handler.cc (right): https://codereview.chromium.org/2629573004/diff/240001/chrome/browser/ui/webui/webapks_handler.cc#newcode1 chrome/browser/ui/webui/webapks_handler.cc:1: // ...
9 months ago (2017-01-24 18:35:32 UTC) #53
Dan Beam (no longer on Chrome)
https://codereview.chromium.org/2629573004/diff/240001/components/webapks_ui/resources/about_webapks.css File components/webapks_ui/resources/about_webapks.css (right): https://codereview.chromium.org/2629573004/diff/240001/components/webapks_ui/resources/about_webapks.css#newcode16 components/webapks_ui/resources/about_webapks.css:16: text-align: left; how does this work in RTL? https://codereview.chromium.org/2629573004/diff/240001/components/webapks_ui/resources/about_webapks.html ...
9 months ago (2017-01-24 18:41:50 UTC) #54
gonzalon
Thanks for reviewing! https://codereview.chromium.org/2629573004/diff/240001/chrome/browser/ui/webui/webapks_handler.cc File chrome/browser/ui/webui/webapks_handler.cc (right): https://codereview.chromium.org/2629573004/diff/240001/chrome/browser/ui/webui/webapks_handler.cc#newcode1 chrome/browser/ui/webui/webapks_handler.cc:1: // Copyright (c) 2017 The Chromium ...
9 months ago (2017-01-24 20:43:29 UTC) #55
Xi Han
lgtm with nits. https://codereview.chromium.org/2629573004/diff/240001/chrome/browser/android/shortcut_helper.h File chrome/browser/android/shortcut_helper.h (right): https://codereview.chromium.org/2629573004/diff/240001/chrome/browser/android/shortcut_helper.h#newcode30 chrome/browser/android/shortcut_helper.h:30: WebApkInfoCallback; The new style is: using ...
9 months ago (2017-01-24 20:44:46 UTC) #56
gonzalon
https://codereview.chromium.org/2629573004/diff/240001/chrome/browser/android/shortcut_helper.h File chrome/browser/android/shortcut_helper.h (right): https://codereview.chromium.org/2629573004/diff/240001/chrome/browser/android/shortcut_helper.h#newcode30 chrome/browser/android/shortcut_helper.h:30: WebApkInfoCallback; On 2017/01/24 20:44:46, Xi Han wrote: > The ...
9 months ago (2017-01-24 20:54:52 UTC) #57
Dan Beam (no longer on Chrome)
lgtm https://codereview.chromium.org/2629573004/diff/280001/components/webapks_ui/resources/about_webapks.css File components/webapks_ui/resources/about_webapks.css (right): https://codereview.chromium.org/2629573004/diff/280001/components/webapks_ui/resources/about_webapks.css#newcode1 components/webapks_ui/resources/about_webapks.css:1: /* Copyright (c) 2017 The Chromium Authors. All ...
8 months, 4 weeks ago (2017-01-24 22:30:28 UTC) #58
gonzalon
https://codereview.chromium.org/2629573004/diff/280001/components/webapks_ui/resources/about_webapks.css File components/webapks_ui/resources/about_webapks.css (right): https://codereview.chromium.org/2629573004/diff/280001/components/webapks_ui/resources/about_webapks.css#newcode1 components/webapks_ui/resources/about_webapks.css:1: /* Copyright (c) 2017 The Chromium Authors. All rights ...
8 months, 4 weeks ago (2017-01-24 22:54:22 UTC) #59
gonzalon
jam@chromium.org: Hi! I added a new directory on the components folder for Web APKs. dbean@ ...
8 months, 4 weeks ago (2017-01-24 23:03:05 UTC) #61
dominickn
webapps / webapks lgtm % nits. Thanks for making the move-only changes: this code is ...
8 months, 4 weeks ago (2017-01-24 23:15:20 UTC) #64
gonzalon
https://codereview.chromium.org/2629573004/diff/300001/chrome/browser/android/shortcut_helper.cc File chrome/browser/android/shortcut_helper.cc (right): https://codereview.chromium.org/2629573004/diff/300001/chrome/browser/android/shortcut_helper.cc#newcode7 chrome/browser/android/shortcut_helper.cc:7: #include <jni.h> On 2017/01/24 23:15:20, dominickn wrote: > Nit: ...
8 months, 4 weeks ago (2017-01-24 23:37:56 UTC) #65
jam
On 2017/01/24 23:03:05, gonzalon wrote: > mailto:jam@chromium.org: > > Hi! > > I added a ...
8 months, 4 weeks ago (2017-01-25 00:09:35 UTC) #66
gonzalon
Moved the html, css and js to src/chrome/browser/resources
8 months, 4 weeks ago (2017-01-25 16:49:42 UTC) #68
Dan Beam (no longer on Chrome)
I'm also an owner of chrome/browser/resources/ so jam@ may not need to review the files ...
8 months, 4 weeks ago (2017-01-25 23:27:34 UTC) #69
jam
On 2017/01/25 16:49:42, gonzalon wrote: > Moved the html, css and js to src/chrome/browser/resources thanks, ...
8 months, 4 weeks ago (2017-01-25 23:59:56 UTC) #70
gonzalon
Fixed the comments and added the closure compiler file. https://codereview.chromium.org/2629573004/diff/340001/chrome/browser/resources/webapks/about_webapks.html File chrome/browser/resources/webapks/about_webapks.html (right): https://codereview.chromium.org/2629573004/diff/340001/chrome/browser/resources/webapks/about_webapks.html#newcode7 chrome/browser/resources/webapks/about_webapks.html:7: ...
8 months, 4 weeks ago (2017-01-26 17:02:49 UTC) #75
dominickn
https://codereview.chromium.org/2629573004/diff/380001/chrome/browser/android/webapk/webapk_info.h File chrome/browser/android/webapk/webapk_info.h (right): https://codereview.chromium.org/2629573004/diff/380001/chrome/browser/android/webapk/webapk_info.h#newcode19 chrome/browser/android/webapk/webapk_info.h:19: WebApkInfo(std::string&& short_name, Sorry! One last thing: remove && from ...
8 months, 3 weeks ago (2017-01-27 05:42:33 UTC) #78
pkotwicz
In retrospect the failure that you are seeing on the bots was a simple one ...
8 months, 3 weeks ago (2017-01-27 16:59:24 UTC) #79
gonzalon
On 2017/01/27 16:59:24, pkotwicz wrote: > In retrospect the failure that you are seeing on ...
8 months, 3 weeks ago (2017-01-27 19:25:10 UTC) #80
gonzalon
https://codereview.chromium.org/2629573004/diff/380001/chrome/browser/android/webapk/webapk_info.h File chrome/browser/android/webapk/webapk_info.h (right): https://codereview.chromium.org/2629573004/diff/380001/chrome/browser/android/webapk/webapk_info.h#newcode19 chrome/browser/android/webapk/webapk_info.h:19: WebApkInfo(std::string&& short_name, On 2017/01/27 05:42:33, dominickn wrote: > Sorry! ...
8 months, 3 weeks ago (2017-01-27 19:25:25 UTC) #81
Dan Beam (no longer on Chrome)
https://codereview.chromium.org/2629573004/diff/400001/chrome/browser/android/webapk/webapk_info.h File chrome/browser/android/webapk/webapk_info.h (right): https://codereview.chromium.org/2629573004/diff/400001/chrome/browser/android/webapk/webapk_info.h#newcode19 chrome/browser/android/webapk/webapk_info.h:19: WebApkInfo(std::string short_name, don't we want these to be const ...
8 months, 3 weeks ago (2017-01-27 21:52:15 UTC) #84
dominickn
https://codereview.chromium.org/2629573004/diff/400001/chrome/browser/android/webapk/webapk_info.h File chrome/browser/android/webapk/webapk_info.h (right): https://codereview.chromium.org/2629573004/diff/400001/chrome/browser/android/webapk/webapk_info.h#newcode19 chrome/browser/android/webapk/webapk_info.h:19: WebApkInfo(std::string short_name, On 2017/01/27 21:52:14, Dan Beam wrote: > ...
8 months, 3 weeks ago (2017-01-27 22:31:05 UTC) #87
Dan Beam (no longer on Chrome)
https://codereview.chromium.org/2629573004/diff/400001/chrome/browser/android/webapk/webapk_info.h File chrome/browser/android/webapk/webapk_info.h (right): https://codereview.chromium.org/2629573004/diff/400001/chrome/browser/android/webapk/webapk_info.h#newcode19 chrome/browser/android/webapk/webapk_info.h:19: WebApkInfo(std::string short_name, On 2017/01/27 22:31:05, dominickn wrote: > On ...
8 months, 3 weeks ago (2017-01-27 22:40:22 UTC) #88
gonzalon
https://codereview.chromium.org/2629573004/diff/400001/chrome/browser/resources/BUILD.gn File chrome/browser/resources/BUILD.gn (right): https://codereview.chromium.org/2629573004/diff/400001/chrome/browser/resources/BUILD.gn#newcode154 chrome/browser/resources/BUILD.gn:154: grit("webapks_ui_resources") { On 2017/01/27 21:52:14, Dan Beam wrote: > ...
8 months, 3 weeks ago (2017-01-27 22:45:03 UTC) #89
Dan Beam (no longer on Chrome)
lgtm
8 months, 3 weeks ago (2017-01-27 22:46:42 UTC) #90
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2629573004/440001
8 months, 3 weeks ago (2017-01-30 17:59:55 UTC) #101
commit-bot: I haz the power
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_presubmit/builds/353225)
8 months, 3 weeks ago (2017-01-30 18:09:24 UTC) #103
gonzalon
jam@chromium.org: Hi again! I'm missing OWNER approval for some build files, would you mind taking ...
8 months, 3 weeks ago (2017-01-30 18:21:53 UTC) #105
pkotwicz
jam@: Ping!
8 months, 2 weeks ago (2017-02-03 15:37:12 UTC) #106
jam
lgtm
8 months, 2 weeks ago (2017-02-06 18:13:03 UTC) #107
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2629573004/440001
8 months, 2 weeks ago (2017-02-07 15:34:23 UTC) #109
commit-bot: I haz the power
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_swarming_rel/builds/113987)
8 months, 2 weeks ago (2017-02-07 15:56:59 UTC) #111
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2629573004/440001
8 months, 2 weeks ago (2017-02-07 16:20:34 UTC) #113
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2629573004/460001
8 months, 2 weeks ago (2017-02-07 16:32:47 UTC) #116
commit-bot: I haz the power
Failed to apply patch for chrome/common/url_constants.h: While running git apply --index -p1; error: patch failed: ...
8 months, 2 weeks ago (2017-02-07 17:24:49 UTC) #118
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2629573004/460001
8 months, 2 weeks ago (2017-02-07 19:18:44 UTC) #120
commit-bot: I haz the power
Failed to apply patch for chrome/common/url_constants.h: While running git apply --index -p1; error: patch failed: ...
8 months, 2 weeks ago (2017-02-07 19:25:44 UTC) #122
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2629573004/480001
8 months, 2 weeks ago (2017-02-07 19:37:26 UTC) #125
commit-bot: I haz the power
8 months, 2 weeks ago (2017-02-07 22:31:48 UTC) #128
Message was sent while issue was closed.
Committed patchset #25 (id:480001) as
https://chromium.googlesource.com/chromium/src/+/87192d7728d4f0829ba7eba2c49b...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa