Sounds like this depends on  issue 794315 .

Alex, do you think you can help with this for M67?

Alex has work in progress for this.  It may not be a strict launch blocker, but it would be desirable to lock hosted apps to their respective origins in M67.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71426a90ee24d449971c658cecbdb2fb2d713e2c

commit 71426a90ee24d449971c658cecbdb2fb2d713e2c
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Fri Aug 17 00:14:43 2018

Lock hosted apps to their underlying web origin.

Previously, hosted apps were exempt from LockToOrigin() even in
--site-per-process mode.  That meant that hosted apps were not subject
to enforcements such as not allowing access to cookies, passwords, or
local storage of other sites.  Worse, it meant that hosted apps could
arbitrarily share a process (e.g., when over process limit), even if
they covered different web sites with --site-per-process.

This CL starts locking hosted apps to their underlying web origin.  If
a frame commits a navigation to URL http://foo.com, which is part of a
hosted app X's web extent, the process for that frame will be locked
to http://foo.com.  Note that the SiteInstance for this frame will
still use a site URL based on the effective URL (i.e.,
chrome-extension://<ext_id_for_X>/), but the origin lock will not be
based on effective URLs.  This requires plumbing to compute the origin
lock as a site URL that does not use an effective URL, and to plumb it
into various places that make process model decisions, such as
RPHI::IsSuitableHost().

Bug:  811939 ,  794315 ,  791796 
Change-Id: Icc9b3c0a04253e581ea35953f3c566308305db59
Reviewed-on: https://chromium-review.googlesource.com/959346
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583895}
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/chrome/browser/ui/extensions/hosted_app_browsertest.cc
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/content/browser/browsing_instance.cc
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/content/browser/child_process_security_policy_impl.cc
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/content/browser/frame_host/navigator_impl_unittest.cc
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/content/browser/isolated_origin_browsertest.cc
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/content/browser/renderer_host/render_process_host_unittest.cc
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/content/browser/site_instance_impl.cc
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/content/browser/site_instance_impl.h
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/content/browser/site_instance_impl_unittest.cc
[modify] https://crrev.com/71426a90ee24d449971c658cecbdb2fb2d713e2c/content/public/browser/site_instance.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/218dcacf7e8891fcad111eaffcfe86794ce79b40

commit 218dcacf7e8891fcad111eaffcfe86794ce79b40
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Fri Aug 17 23:12:44 2018

Revert "Lock hosted apps to their underlying web origin."

This reverts commit 71426a90ee24d449971c658cecbdb2fb2d713e2c.

Reason for revert: Causing cookies and localStorage renderer kills involving the Drive app. I found a local repro where an app covers both http and https versions of a site, and a renderer-initiated navigation from http to https does not trigger a process transfer as it should.

Original change's description:
> Lock hosted apps to their underlying web origin.
> 
> Previously, hosted apps were exempt from LockToOrigin() even in
> --site-per-process mode.  That meant that hosted apps were not subject
> to enforcements such as not allowing access to cookies, passwords, or
> local storage of other sites.  Worse, it meant that hosted apps could
> arbitrarily share a process (e.g., when over process limit), even if
> they covered different web sites with --site-per-process.
> 
> This CL starts locking hosted apps to their underlying web origin.  If
> a frame commits a navigation to URL http://foo.com, which is part of a
> hosted app X's web extent, the process for that frame will be locked
> to http://foo.com.  Note that the SiteInstance for this frame will
> still use a site URL based on the effective URL (i.e.,
> chrome-extension://<ext_id_for_X>/), but the origin lock will not be
> based on effective URLs.  This requires plumbing to compute the origin
> lock as a site URL that does not use an effective URL, and to plumb it
> into various places that make process model decisions, such as
> RPHI::IsSuitableHost().
> 
> Bug:  811939 ,  794315 ,  791796 
> Change-Id: Icc9b3c0a04253e581ea35953f3c566308305db59
> Reviewed-on: https://chromium-review.googlesource.com/959346
> Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
> Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> Reviewed-by: Charlie Reis <creis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583895}

TBR=creis@chromium.org,rdevlin.cronin@chromium.org,alexmos@chromium.org

Change-Id: Ie459ef9b61eb78b6dad44e253768d8c4234e6abf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  811939 ,  794315 ,  791796 
Reviewed-on: https://chromium-review.googlesource.com/1180308
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584231}
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/chrome/browser/ui/extensions/hosted_app_browsertest.cc
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/content/browser/browsing_instance.cc
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/content/browser/child_process_security_policy_impl.cc
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/content/browser/frame_host/navigator_impl_unittest.cc
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/content/browser/isolated_origin_browsertest.cc
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/content/browser/renderer_host/render_process_host_unittest.cc
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/content/browser/site_instance_impl.cc
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/content/browser/site_instance_impl.h
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/content/browser/site_instance_impl_unittest.cc
[modify] https://crrev.com/218dcacf7e8891fcad111eaffcfe86794ce79b40/content/public/browser/site_instance.h

I've reverted my initial attempt at locking hosted apps to an origin because today's canary (70.0.3525.0) that included r583895 had a fair number of renderer kills involving the Google Drive app, e.g.:
https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27%5BRenderer+kill+79%5D+content%3A%3ARenderFrameMessageFilter%3A%3AGetCookies%27+AND+product.Version%3D%2770.0.3525.0%27&stbtiq=&reportid=&index=0

I investigated a bit and found that when a hosted app covers multiple sites, say foo.com and bar.com, renderer-initiated navigations from foo.com to bar.com did not cause a process transfer with my CL as they now should.  I think the problem is that we decide to transfer and try to create a new related SiteInstance in DetermineSiteInstanceForURL, but since both URLs resolve to the same effective site URL of chrome-extension://blah, we find and return the *old* SiteInstance and stay in the old process.  The next request to manipulate cookies or localStorage will then kill the renderer.

For Drive, crash data suggests this involved transferring from http://drive.google.com to https://drive.google.com.  The newer Drive app (14.3_0) manifest doesn't actually cover any sites in its web extent anymore (yay), but the older app (14.1_0) does:
      "urls": [ "http://docs.google.com/", "http://drive.google.com/", "https://docs.google.com/", "https://drive.google.com/" ]

I've reproed the localStorage kill with the older Drive app that I still had in an older profile, just by going to http://drive.google.com.

I'll be OOO next week and will fix this and reland once I'm back.  I think this might involve augmenting site_url with lock_url when effective URLs are used, similarly to what I had to do for SiteProcessMap in PS17 of r583895.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5f926a551d7803b47517c7f928448ffc944d50d0

commit 5f926a551d7803b47517c7f928448ffc944d50d0
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Wed Aug 29 20:57:30 2018

Reland: lock hosted apps to their underlying web origin.

This is a reland of https://crrev.com/c/959346 with an additional fix
for cross-site navigations between sites covered by the same hosted
app.  The fix modifies site URLs for effective URLs to also include
the non-translated site in the hash.  This ensures that when multiple
sites translate to the same effective app URL, they still have
distinct site URLs.  This ensures proper process swaps when navigating
among these sites.

Original description:

Previously, hosted apps were exempt from LockToOrigin() even in
--site-per-process mode.  That meant that hosted apps were not subject
to enforcements such as not allowing access to cookies, passwords, or
local storage of other sites.  Worse, it meant that hosted apps could
arbitrarily share a process (e.g., when over process limit), even if
they covered different web sites with --site-per-process.

This CL starts locking hosted apps to their underlying web origin.  If
a frame commits a navigation to URL http://foo.com, which is part of a
hosted app X's web extent, the process for that frame will be locked
to http://foo.com.  Note that the SiteInstance for this frame will
still use a site URL based on the effective URL (i.e.,
chrome-extension://<ext_id_for_X>/), but the origin lock will not be
based on effective URLs.  This requires plumbing to compute the origin
lock as a site URL that does not use an effective URL, and to plumb it
into various places that make process model decisions, such as
RPHI::IsSuitableHost().

Bug:  811939 ,  794315 ,  791796 
Change-Id: I32cc54f3ee12ed5762272e5cfe43bd5eca94e2eb
Reviewed-on: https://chromium-review.googlesource.com/959346
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#583895}
Reviewed-on: https://chromium-review.googlesource.com/1194724
Cr-Commit-Position: refs/heads/master@{#587295}
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/chrome/browser/ui/extensions/hosted_app_browsertest.cc
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/content/browser/browsing_instance.cc
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/content/browser/child_process_security_policy_impl.cc
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/content/browser/frame_host/navigator_impl_unittest.cc
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/content/browser/frame_host/render_frame_host_manager_unittest.cc
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/content/browser/isolated_origin_browsertest.cc
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/content/browser/renderer_host/render_process_host_unittest.cc
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/content/browser/site_instance_impl.cc
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/content/browser/site_instance_impl.h
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/content/browser/site_instance_impl_unittest.cc
[modify] https://crrev.com/5f926a551d7803b47517c7f928448ffc944d50d0/content/public/browser/site_instance.h

The reland in r587295 is in today's 70.0.3537.0 canary, and so far there aren't any cookie kills in that version.  I'll continue monitoring, but hopefully the issue from #8 is resolved.

I haven't noticed anything suspicious yet when looking at other crashes.  There are a few for renderer kill 205 (ValidateDidCommitParams), but looking at killed_process_origin_lock they are all for error page isolation. 

Checked crashes again today.  There were 7 cookie kills that came up in 70.0.3537.0+ [1], but they're all on Android and from only two clients.  These kills are in Android's canary site-per-process trial, but I think they're probably a separate issue unrelated to the change here - Android doesn't even have hosted apps, the Instant effective URL code in ChromeContentBrowserClient::GetEffectiveURL() is compiled out on Android, and the killed_process_origin_lock / requested_site_url crash keys look like normal sites rather than effective URLs.  

Based on this, I think I'll go ahead and close this for now, but let's keep watching for regressions.  BTW, this made the M70 branch, which was at r587811.

[1] https://crash.corp.google.com/browse?q=product_name+LIKE+%27%25Chrome%25%27+AND+expanded_custom_data.ChromeCrashProto.channel%3D%27canary%27+AND+product.Version%3E%3D%2770.0.3537.0%27+AND+expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27%5BRenderer+kill+79%5D+content%3A%3ARenderFrameMessageFilter%3A%3AGetCookies%27

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot