|
|
Description[Android] Enable WebAPK to have multiple intent filters
This CL:
- Changes AndroidManifest.xml MoTemplate to support multiple intent filters
- Moves the MoTemplate configuration to external .json files
- Adds logic to rewrite URLs from deep links to the scope URL's scheme and host
name. This handles the case of:
1) The WebAPK has intent filters for several TLDs (e.g.
https://maps.google.ca,
https://maps.google.fr, ...)
2) It is desired to change the TLD of the intent URL to '.com' so that all
incoming intent URLs fall under one service worker.
BUG=734131, 734136
Review-Url: https://codereview.chromium.org/2956193002
Cr-Commit-Position: refs/heads/master@{#490275}
Committed: https://chromium.googlesource.com/chromium/src/+/6c7ac7c0265c50a2da530c21f040a90413a2b080
Patch Set 1 : Merge branch 'rewriting0' into rewriting #
Total comments: 13
Patch Set 2 : Merge branch 'mustache' into rewriting #
Total comments: 11
Patch Set 3 : Merge branch 'rewriting0' into rewriting #
Total comments: 1
Patch Set 4 : Merge branch 'master' into rewriting #Patch Set 5 : Merge branch 'master' into rewriting #Patch Set 6 : Merge branch 'master' into rewriting #Messages
Total messages: 32 (10 generated)
Patchset #1 (id:1) has been deleted
Description was changed from ========== [Android] Enable WebAPK to have multiple intent filters This CL: - Changes AndroidManifest.xml MoTemplate to support multiple intent filters - Moves the MoTemplate configuration to external .json files - Adds logic to rewrite URLs from deep links to the scope URL's scheme and host name. This handles the case of: 1) The WebAPK has intent filters for several TLDs (e.g. https://maps.google.ca, https://maps.google.fr ...) 2) Changing the TLD of the intent URL to '.com' so that all incoming intent URLs fall under one intent filter BUG=734131,734136 ========== to ========== [Android] Enable WebAPK to have multiple intent filters This CL: - Changes AndroidManifest.xml MoTemplate to support multiple intent filters - Moves the MoTemplate configuration to external .json files - Adds logic to rewrite URLs from deep links to the scope URL's scheme and host name. This handles the case of: 1) The WebAPK has intent filters for several TLDs (e.g. https://maps.google.ca, https://maps.google.fr, ...) 2) It is desired to change the TLD of the intent URL to '.com' so that all incoming intent URLs fall under one service worker. BUG=734131,734136 ==========
pkotwicz@chromium.org changed reviewers: + yfriedman@chromium.org
Yaron, can you please take a look? I change AndroidManifest.xml to be in the MoTemplate (previously called Handlebars) format instead of in the Mustache format. The MoTemplate format is "inspired by Mustache" but it is not Mustache. I changed the format of AndroidManifest.xml because converting from the MoTemplate format to Mustache is easier than the reverse.
https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/AndroidManifest.xml (right): https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/AndroidManifest.xml:27: {{#intent_filters:intent_filters}} I'm not familiar with this syntax but have you verified it works with the server implementation?
https://codereview.chromium.org/2956193002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java (right): https://codereview.chromium.org/2956193002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java:190: // The WebAPK may have been launched as a result of an intent filter for a different scheme this is an interesting place to put it - I would've thought this goes in the webapk itself. I'm not saying this is wrong, but unsure and would like to hear your thoughts on why here. https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/AndroidManifest.xml (right): https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/AndroidManifest.xml:49: <!-- Hashes of icons should be taken of the icons as they are available on the web. The icon Use mustache comment {{! instaed
yfriedman@chromium.org changed reviewers: + hartmanng@chromium.org
+Glenn cause this will have to be versioned properly on the server
+Glenn cause this will have to be versioned properly on the server
yfriedman@chromium.org changed reviewers: + palmer@chromium.org
+palmer as I know any URL munging (although you're nicely using built-ins) raises his alarms as much or more than mine :) CL description doesn't mention https either https://codereview.chromium.org/2956193002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java (right): https://codereview.chromium.org/2956193002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java:190: // The WebAPK may have been launched as a result of an intent filter for a different scheme On 2017/06/28 16:59:39, Yaron wrote: > this is an interesting place to put it - I would've thought this goes in the > webapk itself. I'm not saying this is wrong, but unsure and would like to hear > your thoughts on why here. +palmer
On 2017/06/28 17:00:22, Yaron wrote: > +Glenn cause this will have to be versioned properly on the server Versioning shouldn't be an issue any more than any normal shell update. the {{#intent_filters}} section (once the changes are made from my comment) will just get stripped out of the template if it's not specified on the server side. https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/AndroidManifest.xml (right): https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/AndroidManifest.xml:27: {{#intent_filters:intent_filters}} On 2017/06/28 16:55:40, Yaron wrote: > I'm not familiar with this syntax but have you verified it works with the server > implementation? it doesn't as-is. The changes to make it work on the server are easy, but I can't promise it'll still work on the client afterwards... Changes required: - the server doesn't like {{#intent_filters:intent_filters}} ("interleaved closing tag: intent_filters"), but seems to work fine with just {{#intent_filters}}. - the "inner" variables ("intent_filters.scope_url_scheme", etc) don't expect the "intent_filters." prefix on the server implementation. They seem to just want "scope_url_scheme".
https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/AndroidManifest.xml (right): https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/AndroidManifest.xml:27: {{#intent_filters:intent_filters}} On 2017/06/28 18:24:33, hartmanng wrote: > On 2017/06/28 16:55:40, Yaron wrote: > > I'm not familiar with this syntax but have you verified it works with the > server > > implementation? > > it doesn't as-is. The changes to make it work on the server are easy, but I > can't promise it'll still work on the client afterwards... > > Changes required: > - the server doesn't like {{#intent_filters:intent_filters}} ("interleaved > closing tag: intent_filters"), but seems to work fine with just > {{#intent_filters}}. > - the "inner" variables ("intent_filters.scope_url_scheme", etc) don't expect > the "intent_filters." prefix on the server implementation. They seem to just > want "scope_url_scheme". Ya, those match my expectations. I would think that it would work on the client but I've been (unpleasantly) surprised by its impl before
https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/manifest_processor.py (right): https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/manifest_processor.py:10: To convert Motemplate to "Mustache template" replace: Oh I just saw that explicitly called this out :/ Per offline discussion, wondering if it's easier to just get pystache into chromium
Can everyone please take another look? Now this CL uses real Mustache (and the server won't have to do any processing) https://codereview.chromium.org/2956193002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java (right): https://codereview.chromium.org/2956193002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java:190: // The WebAPK may have been launched as a result of an intent filter for a different scheme I put the logic here because the goal is for WebAPKs to be as small as possible. The WebAPK itself currently does not read the "scope" meta data property. We determine whether to navigate the WebAPK as a result of a given WebAPK intent in WebApkInfo as well In my opinion, we should keep this logic in Chrome for now. We can move the logic into the WebAPK if we need to. (I understand the argument for putting the code in the WebAPK) https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/AndroidManifest.xml (right): https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/AndroidManifest.xml:27: {{#intent_filters:intent_filters}} It doesn't. The server will have the process the AndroidManifest.xml to convert the MoTemplate syntax to Mustache. Converting from MoTemplate to Mustache is easy. (Remove the text before the ':') For the sake of clarity, MoTemplate rejects the Mustache syntax that you are familiar with https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/AndroidManifest.xml:49: <!-- Hashes of icons should be taken of the icons as they are available on the web. The icon Done. Does Android keep XML comments when it constructs the .apk?
AndroidManifest.xml l-g-t-m https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/AndroidManifest.xml (right): https://codereview.chromium.org/2956193002/diff/20001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/AndroidManifest.xml:27: {{#intent_filters:intent_filters}} On 2017/06/28 20:45:06, pkotwicz wrote: > It doesn't. The server will have the process the AndroidManifest.xml to convert > the MoTemplate syntax to Mustache. > > Converting from MoTemplate to Mustache is easy. (Remove the text before the ':') > > For the sake of clarity, MoTemplate rejects the Mustache syntax that you are > familiar with Just to be completely explicit here: the part of this comment that says "The server will have the process the AndroidManifest.xml to convert the MoTemplate syntax to Mustache" is no longer true in the most recent patchset, correct?
https://codereview.chromium.org/2956193002/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java (right): https://codereview.chromium.org/2956193002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java:192: // name. This is where the rubber hits the road, right? How do we know at this point in the code that it's acceptable (to Chrome, to the site(s) in question) to do these mappings?
https://codereview.chromium.org/2956193002/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java (right): https://codereview.chromium.org/2956193002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java:192: // name. I think that Yaron was asking whether doing the URL rewriting using android.net.Uri was equivalent to doing the URL rewriting using GURL::ReplaceComponents() The site in question would specify that they want this mapping in the Web Manifest that their site links to (https://www.w3.org/TR/appmanifest/) The way that this is specified in the Web Manifest is not spec'ed yet. We won't do any URL rewriting unless the site explicitly requests it. It is currently impossible for WebAPKs installed via "Add-to-homescreen" in the app menu (or any other manner) to have an intent filter different than the WebAPK scope. We have no plans of changing this. As an experiment, we want to allow a few first party Google apps to have associated WebAPKs for which the URL host is rewritten. We will package these WebAPKs by hand (and are doing the URL rewriting at the explicit request of those first party apps). More details are at https://docs.google.com/a/google.com/document/d/1il9QhxNeHEQG-vsgCnWV_f-_8Bmr...
i assume a shell apk version increase will be done as this gets closer to landing? We'll need it to increment for the server to be happy https://codereview.chromium.org/2956193002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java (right): https://codereview.chromium.org/2956193002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java:190: // The WebAPK may have been launched as a result of an intent filter for a different scheme On 2017/06/28 20:45:06, pkotwicz wrote: > I put the logic here because the goal is for WebAPKs to be as small as possible. > The WebAPK itself currently does not read the "scope" meta data property. We > determine whether to navigate the WebAPK as a result of a given WebAPK intent in > WebApkInfo as well > > In my opinion, we should keep this logic in Chrome for now. We can move the > logic into the WebAPK if we need to. (I understand the argument for putting the > code in the WebAPK) Ya, I guess I didn't really realize that atleast the Uri field in WebApkInfo/WebAppInfo really describes the initial launch intent into the webapk. I suppose this is actually a decent spot for this. Once you're in the actual webapk - should navigations within it also be rewritten? I expect not as the developer has control over this. Also, adding this to WebApk itself would yield the same result. https://codereview.chromium.org/2956193002/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java (right): https://codereview.chromium.org/2956193002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java:192: // name. On 2017/06/28 22:26:21, pkotwicz wrote: > I think that Yaron was asking whether doing the URL rewriting using > android.net.Uri was equivalent to doing the URL rewriting using > GURL::ReplaceComponents() Yes, that was one part. It sounds like it's safe. The other was whether it should be in the MainActivity of WebApk vs here which I asked elsewhere. > > The site in question would specify that they want this mapping in the Web > Manifest that their site links to (https://www.w3.org/TR/appmanifest/) The way > that this is specified in the Web Manifest is not spec'ed yet. We won't do any > URL rewriting unless the site explicitly requests it. > > It is currently impossible for WebAPKs installed via "Add-to-homescreen" in the > app menu (or any other manner) to have an intent filter different than the > WebAPK scope. We have no plans of changing this. > > As an experiment, we want to allow a few first party Google apps to have > associated WebAPKs for which the URL host is rewritten. We will package these > WebAPKs by hand (and are doing the URL rewriting at the explicit request of > those first party apps). More details are at > https://docs.google.com/a/google.com/document/d/1il9QhxNeHEQG-vsgCnWV_f-_8Bmr... https://codereview.chromium.org/2956193002/diff/40001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/BUILD.gn (right): https://codereview.chromium.org/2956193002/diff/40001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/BUILD.gn:20: extra_variables = [ "shell_apk_version=$template_shell_apk_version" ] Did you keep this around for testing purposes? They're all set the same way.
Yaron, can you please take another look? I will modify shell_apk_version.gni once the third_party CL lands https://codereview.chromium.org/2956193002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java (right): https://codereview.chromium.org/2956193002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java:190: // The WebAPK may have been launched as a result of an intent filter for a different scheme No, navigations within the WebAPK should not be rewritten https://codereview.chromium.org/2956193002/diff/40001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/BUILD.gn (right): https://codereview.chromium.org/2956193002/diff/40001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/BUILD.gn:20: extra_variables = [ "shell_apk_version=$template_shell_apk_version" ] $template_shell_apk_version comes from shell_apk_version.gni I didn't want to use java_cpp_template() to process the config files (I didn't want to templatize the template-config-files)
https://codereview.chromium.org/2956193002/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java (right): https://codereview.chromium.org/2956193002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java:192: // name. On 2017/06/29 02:15:07, Yaron wrote: > On 2017/06/28 22:26:21, pkotwicz wrote: > > I think that Yaron was asking whether doing the URL rewriting using > > android.net.Uri was equivalent to doing the URL rewriting using > > GURL::ReplaceComponents() > > Yes, that was one part. It sounds like it's safe. > The other was whether it should be in the MainActivity of WebApk vs here which I > asked elsewhere. > > > > > The site in question would specify that they want this mapping in the Web > > Manifest that their site links to (https://www.w3.org/TR/appmanifest/) The way > > that this is specified in the Web Manifest is not spec'ed yet. We won't do any > > URL rewriting unless the site explicitly requests it. > > > > It is currently impossible for WebAPKs installed via "Add-to-homescreen" in > the > > app menu (or any other manner) to have an intent filter different than the > > WebAPK scope. We have no plans of changing this. > > > > As an experiment, we want to allow a few first party Google apps to have > > associated WebAPKs for which the URL host is rewritten. We will package these > > WebAPKs by hand (and are doing the URL rewriting at the explicit request of > > those first party apps). More details are at > > > https://docs.google.com/a/google.com/document/d/1il9QhxNeHEQG-vsgCnWV_f-_8Bmr... > So playing this out - what happens if you're in a webapk, click a link that's out of the primary scope but would hit the intent filters? The link click would run ExternalNavigationHandler, the url would match a secondary intent filter, we see we're already in the webapk and we wouldn't rewrite it. So urls which would launch a webapk get rewritten but within it but off the main origin don't. I don't think it would break but it's a little strange? What do you think? https://codereview.chromium.org/2956193002/diff/40001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/BUILD.gn (right): https://codereview.chromium.org/2956193002/diff/40001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/BUILD.gn:20: extra_variables = [ "shell_apk_version=$template_shell_apk_version" ] On 2017/06/29 04:46:41, pkotwicz wrote: > $template_shell_apk_version comes from shell_apk_version.gni I didn't want to > use java_cpp_template() to process the config files (I didn't want to templatize > the template-config-files) sgtm :) https://codereview.chromium.org/2956193002/diff/40001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/bound_manifest_config.json (right): https://codereview.chromium.org/2956193002/diff/40001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/bound_manifest_config.json:4: "intent_filters": { so does this "Just work" if you have an array of them? The template would be expanded multiple times?
https://codereview.chromium.org/2956193002/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java (right): https://codereview.chromium.org/2956193002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java:192: // name. The goal of the URL rewriting is to enable a web app to run within a single top level domain. This requires cooperation on the part of the web app author. An option (that I prefer) involves not rewriting top level domains and giving the web app the responsibility to register a service worker for each likely top level domain. My understanding is that for a given user there are only two likely top level domains .country and .com The web app would register a service worker for likely top level domains via a pagelet. The web app can do this only the first time that it is launched. One complication is if the webapp wants to have different behavior when running in a Chrome tab and when running as a WebAPK. For instance, the webapp may not want to register a service worker when running in a Chrome tab (each JavaScript byte that the user has to download makes the page load slower)
https://codereview.chromium.org/2956193002/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java (right): https://codereview.chromium.org/2956193002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java:192: // name. On 2017/07/11 14:31:38, pkotwicz wrote: > The goal of the URL rewriting is to enable a web app to run within a single top > level domain. This requires cooperation on the part of the web app author. > > An option (that I prefer) involves not rewriting top level domains and giving > the web app the responsibility to register a service worker for each likely top > level domain. My understanding is that for a given user there are only two > likely top level domains .country and .com The web app would register a service > worker for likely top level domains via a pagelet. The web app can do this only > the first time that it is launched. One complication is if the webapp wants to > have different behavior when running in a Chrome tab and when running as a > WebAPK. Not sure that's really desirable. > For instance, the webapp may not want to register a service worker when > running in a Chrome tab (each JavaScript byte that the user has to download > makes the page load slower) So thinking more about this, I'm still wondering it makes more sense to move it to the webapk. That way it's an impl detail of the WebApk and chrome is oblivious to it which means you don't have to rationalize it in the context of chrome. Specifically from chrome's pov, the app lives on a single origin and the webapk is just rewriting some requests to be included. The previous justification of scope not being used in the webapk isn't very concerning to me - we're talking about at most a few hundred bytes and reading a member variable. https://codereview.chromium.org/2956193002/diff/40001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/bound_manifest_config.json (right): https://codereview.chromium.org/2956193002/diff/40001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/bound_manifest_config.json:4: "intent_filters": { On 2017/07/10 23:39:35, Yaron wrote: > so does this "Just work" if you have an array of them? The template would be > expanded multiple times? bump
Yaron, can you please take another look? I moved the logic to the shell APK as you have requested
lgtm https://codereview.chromium.org/2956193002/diff/60001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java (right): https://codereview.chromium.org/2956193002/diff/60001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java:95: // scheme and host name. nit: there's an extra space in there :)
The CQ bit was checked by pkotwicz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yfriedman@chromium.org Link to the patchset: https://codereview.chromium.org/2956193002/#ps120001 (title: "Merge branch 'master' into rewriting")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 120001, "attempt_start_ts": 1501212539630260, "parent_rev": "0bd6b7554df79440a983d42c85589238271e0c61", "commit_rev": "13a89569d77c28d0c3bbb579af1ce5fb3abe79a2"}
CQ is committing da patch. Bot data: {"patchset_id": 120001, "attempt_start_ts": 1501212539630260, "parent_rev": "55ebf5a43475f5addddf3fb9cd909b6ea70c9e5c", "commit_rev": "6c7ac7c0265c50a2da530c21f040a90413a2b080"}
Message was sent while issue was closed.
Description was changed from ========== [Android] Enable WebAPK to have multiple intent filters This CL: - Changes AndroidManifest.xml MoTemplate to support multiple intent filters - Moves the MoTemplate configuration to external .json files - Adds logic to rewrite URLs from deep links to the scope URL's scheme and host name. This handles the case of: 1) The WebAPK has intent filters for several TLDs (e.g. https://maps.google.ca, https://maps.google.fr, ...) 2) It is desired to change the TLD of the intent URL to '.com' so that all incoming intent URLs fall under one service worker. BUG=734131,734136 ========== to ========== [Android] Enable WebAPK to have multiple intent filters This CL: - Changes AndroidManifest.xml MoTemplate to support multiple intent filters - Moves the MoTemplate configuration to external .json files - Adds logic to rewrite URLs from deep links to the scope URL's scheme and host name. This handles the case of: 1) The WebAPK has intent filters for several TLDs (e.g. https://maps.google.ca, https://maps.google.fr, ...) 2) It is desired to change the TLD of the intent URL to '.com' so that all incoming intent URLs fall under one service worker. BUG=734131,734136 Review-Url: https://codereview.chromium.org/2956193002 Cr-Commit-Position: refs/heads/master@{#490275} Committed: https://chromium.googlesource.com/chromium/src/+/6c7ac7c0265c50a2da530c21f040... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:120001) as https://chromium.googlesource.com/chromium/src/+/6c7ac7c0265c50a2da530c21f040... |