To reproduce the bug you need to install uBlock Origin extension first:
https://chrome.google.com/webstore/detail/cjpalhdlnbpafiamejdnhcphjbkeiagm

Bisected to r552589 = fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd = https://crrev.com/c/981019 by lukasza@chromium.org
"Make --site-per-process the default on ToT via fieldtrial_testing_config"

The underlying bug was introduced earlier, bisected with --site-per-process command line:
https://chromium.googlesource.com/chromium/src/+log/fe8dd81c..0e7179e2?pretty=fuller
Suspecting r497314 = 59562ccf0ba406d5d0fb76801ed15de125f9d517 = https://crrev.com/c/611102 by nasko@chromium.org
"Implement localStorage origin enforcement."
Landed in 62.0.3197.0

Crash dump from r559802 is attached.
StoragePartitionImpl::OpenLocalStorage seems to send a bad message.

FWIW, the :// entry looks like a bug per se which appeared in Chrome 62:
https://chromium.googlesource.com/chromium/src/+log/2e3125b7..b552f416?pretty=fuller
Should I open a new bug report?

Deleted: r559802 crashdump.zip 639 KB

r559802 crashdump.zip 639 KB Download

As per comment two we have tested this issue and able to reproduce issue on reported chrome version 68.0.3423.2, latest Stable 66.0.3359.181, Beta 67.0.3396.48  & on latest chrome 68.0.3436.0 using Windows 7 &10,Ubuntu 17.10 and Mac 10.13.3. Hence providing bisect information below.

Bisect Info:
================
Good build: 62.0.3196.0
Bad build: 62.0.3197.0

CHANGELOG URL:
https://chromium.googlesource.com/chromium/src/+/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd

suspect: https://chromium.googlesource.com/chromium/src/+/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd

Reviewed-on: https://chromium-review.googlesource.com/981019

@lukasza: Please confirm the issue and help in re-assigning if it is not related to your change.

Thanks!

Thank you very much for reporting this crash and providing repro steps.  The repro steps should be very helpful in creating a regression test and fixing the crash.

I can repro the crash on Windows in 68.0.3436.0 opted into site-per-process via chrome://flags.  My crash report is was: 5c6b9803ef65a859:
- This seems to be a specific instance of issue 797994
- Looking at other crash reports associated with issue 797994 I see that 7% of the '[Renderer kill 123] content::mojom::StoragePartitionServiceStubDispatch::Accept' kills have cjpalhdlnbpafiamejdnhcphjbkeiagm extension (uBlock Origin) installed
- In crash keys I see killed_process_origin_lock=https://derstandard.at/, but no other crash keys got logged (e.g. requested_origin or requested_site_url)

According to Crash reports for the 5% site-per-process experiment on M66 stable, the kill 123 is ranked ~#278.  I think this is not high-enough for the ReleaseBlock-Stable label.

If we ship site-per-process to 100% then (because the balancing effect of the control group will disappear) this kill can raise to rank ~#160.  Therefore we may consider adding Proj-SiteIsolation-LaunchBlocking label, but I think this is also not needed at this time.

Renderer-side of the localStorage request that triggers the kill:

[231152:231152:0521/151855.492483:ERROR:local_storage_cached_area.cc(114)] LocalStorageCachedAreas::ctor; origin = null; origin_ = null; stack = #0 0x7fb835f2dcfc base::debug::StackTrace::StackTrace()
#1 0x7fb833a32171 content::LocalStorageCachedArea::LocalStorageCachedArea()
#2 0x7fb833a3773f content::LocalStorageCachedAreas::GetCachedArea()
#3 0x7fb833a3720e content::LocalStorageCachedAreas::GetCachedArea()
#4 0x7fb833a3a7a4 content::LocalStorageNamespace::CreateStorageArea()
#5 0x7fb82c1b920b blink::StorageNamespace::LocalStorageArea()
#6 0x7fb82c1b552d blink::InspectorDOMStorageAgent::FindStorageArea()
#7 0x7fb82c1b56bd blink::InspectorDOMStorageAgent::getDOMStorageItems()
#8 0x7fb82c1b5c22 blink::InspectorDOMStorageAgent::getDOMStorageItems()
#9 0x7fb82ec74ae8 blink::protocol::DOMStorage::DispatcherImpl::getDOMStorageItems()
#10 0x7fb82ec38df0 blink::protocol::Accessibility::DispatcherImpl::dispatch()
#11 0x7fb82ecb3976 blink::protocol::UberDispatcher::dispatch()
#12 0x7fb82e4544fb blink::InspectorSession::DispatchProtocolMessage()
#13 0x7fb82cc5f722 blink::mojom::blink::DevToolsSessionStubDispatch::Accept()
#14 0x7fb835018262 mojo::InterfaceEndpointClient::HandleValidatedMessage()

And browser-side logs when handling the localStorage request:

[231109:231109:0521/151855.663325:ERROR:child_process_security_policy_impl.cc(1035)] CPSPI::CanAccessDataForOrigin; site_url = ; origin_lock = https://derstandard.at/
[231109:231109:0521/151855.663363:ERROR:storage_partition_impl.cc(850)] StoragePartitionImpl::OpenLocalStorage; origin = null; origin.GetURL() = ; process_id = 4
[231109:231109:0521/151855.663439:ERROR:render_process_host_impl.cc(4356)] Terminating render process for bad Mojo message: Received bad user message: Access denied for localStorage request
[231109:231109:0521/151855.663473:ERROR:bad_message.cc(25)] Terminating renderer for bad IPC message, reason 123

I've just double-checked that there unique (the spec calls them "opaque) origins do not get access to localStorage.  I did this verification using the following steps:
1. Navigate to data:text/html,<html>hello</html> (this is a unique/opaque origin)
2. In DevTools console execute:
   > localStorage["blah"]
   Uncaught DOMException: Failed to read the 'localStorage' property from 'Window': Storage is disabled inside 'data:' URLs.
    at <anonymous>:1:1

I have also tried the following:
1. Navigate to https://webkit.org/demos/frames/sandboxing/
2. In DevTools select the 3rd am-i-sandboxed.html frame (the one where scripting is allowed) and execute:
   > localStorage["blah"]
   Uncaught DOMException: Failed to read the 'localStorage' property from 'Window': The document is sandboxed and lacks the 'allow-same-origin' flag.
    at <anonymous>:1:1

Based on the above, it seems that localStorage is explicitly disabled for unique/opaque origins.  Therefore I think this kill exposes a bug in DevTools - DevTools should not attempt to list localStorage for unique/opaque origins.

dgozman@ or kozyatinskiy@ - could you PTAL and try to tackle this bug from DevTools side?  See #c9 - #c10 above for info about what I think went wrong.

BTW: dcheng@ claims out that localStorage-VS-opaqueOrigin checks are done in 3 different places.  Maybe it would be worth it to consolidate these checks when working on the crash fix?  Not sure myself (I am not familiar with this code / don't have a strong opinion;  maybe an explicit/ad-hoc check in DevTools is also okay).

BTW: Following up on the comment I made in #c7, I think that this is more of a Priority2, than Priority1 bug (feel free to shout and/or override this assessment if you disagree).

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

commit 1600a1f6c38cc1e3e56d6ad5b48ca88f01ea8ab2
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Wed Jun 13 22:40:21 2018

[DevTools] Fix DOM Storage with OOPIFs

- Collect DOM storages from all targets.
- Instantiate InspectorDOMStorage agent for local roots, make
  it work with InspectedFrames.
- Add a test which dumps storage security origins.
- Do some origin checks on the frontend to avoid listing
  "opaque" security origins which do not support storage.
- Check storage capabilities on the backend to avoid
  crasing renderer later.

Bug:  850978 ,  844377 
Change-Id: Ib4e2b444473638d896eb716fb7d9c587f5560335
Reviewed-on: https://chromium-review.googlesource.com/1096283
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567024}
[add] https://crrev.com/1600a1f6c38cc1e3e56d6ad5b48ca88f01ea8ab2/third_party/WebKit/LayoutTests/http/tests/devtools/oopif/oopif-storage-expected.txt
[add] https://crrev.com/1600a1f6c38cc1e3e56d6ad5b48ca88f01ea8ab2/third_party/WebKit/LayoutTests/http/tests/devtools/oopif/oopif-storage.js
[modify] https://crrev.com/1600a1f6c38cc1e3e56d6ad5b48ca88f01ea8ab2/third_party/blink/renderer/devtools/front_end/resources/ApplicationPanelSidebar.js
[modify] https://crrev.com/1600a1f6c38cc1e3e56d6ad5b48ca88f01ea8ab2/third_party/blink/renderer/devtools/front_end/resources/DOMStorageModel.js
[modify] https://crrev.com/1600a1f6c38cc1e3e56d6ad5b48ca88f01ea8ab2/third_party/blink/renderer/devtools/front_end/sdk/TargetManager.js
[modify] https://crrev.com/1600a1f6c38cc1e3e56d6ad5b48ca88f01ea8ab2/third_party/blink/renderer/modules/modules_initializer.cc
[modify] https://crrev.com/1600a1f6c38cc1e3e56d6ad5b48ca88f01ea8ab2/third_party/blink/renderer/modules/storage/inspector_dom_storage_agent.cc
[modify] https://crrev.com/1600a1f6c38cc1e3e56d6ad5b48ca88f01ea8ab2/third_party/blink/renderer/modules/storage/inspector_dom_storage_agent.h

Thanks for the fix!  I've confirmed the "://" entries are gone in 69.0.3460.0, and that  issue 850978  is resolved as well.

dgozman@: Can you request merge to M68 if you think r567024 is safe, since this is a crash fix as well as a functionality fix?  Much appreciated!

For reference, I tried merging this in a local checkout of M68.  There were some merge conflicts with mek@'s https://chromium-review.googlesource.com/c/chromium/src/+/1091926 (r566272).  They're a little subtle, but a quick attempt to address them compiled and did fix the bug.

I guess this boils down to whether dgozman@ thinks this is a safe thing to merge to M68, based on the merge conflict.  We can chat about it on Monday to see if it's worthwhile.

The NextAction date has arrived: 2018-06-18

Requesting merge to 68. This is a moderately sized crash fix, worth merging.

This bug requires manual review: M68 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

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

How safe is this merge overall? Has this been fully verified in canary?

I did verify the fix on Canary in comment 16 and on a local M68 checkout in comment 17, but I'll let dgozman@ comment on the overall safety of it.  Thanks!

This is not the safest fix I've seen, but there have been no issues from Canary for 4 days. I wouldn't normally merge this, but it fixes a renderer crash, so I thought that raises the bar a little bit.

abdulsyed@, PTAL comment #23 and decide whether to take this merge in for M68 or not. Thank you.

For reference, we do see a fair number of the "[Renderer kill 123] content::mojom::StoragePartitionServiceStubDispatch::Accept" crash reports in earlier versions (including the latest M68, which has 19 instances, and the latest M67, which has 6040 instances).

It appears that we haven't seen those crashes since 69.0.3449.0.  This fix landed in 69.0.3460.0, so it's possible something else may have helped as well, but it seems like it might be worth including this to get help with the crashes as well as the usability fix.

https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27%5BRenderer+kill+123%5D+content%3A%3Amojom%3A%3AStoragePartitionServiceStubDispatch%3A%3AAccept%27#-property-selector,productname:1000,productversion:1000,-magicsignature:50,-magicsignature2:50,-stablesignature:50,-magicsignaturesorted:50

We're also seeing more user reports of this (e.g.,  issue 853679 , which was duped into  issue 850978 ).

Thanks for providing more feedback. Per #23,#25, I am fine with taking this merge in 68, and closely monitoring. branch:3440

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

commit e2ce4979a905dfcb953cb56ded2ddc2cbb1bdd4f
Author: Charlie Reis <creis@chromium.org>
Date: Tue Jun 19 22:11:51 2018

Merge to M68: [DevTools] Fix DOM Storage with OOPIFs

- Collect DOM storages from all targets.
- Instantiate InspectorDOMStorage agent for local roots, make
  it work with InspectedFrames.
- Add a test which dumps storage security origins.
- Do some origin checks on the frontend to avoid listing
  "opaque" security origins which do not support storage.
- Check storage capabilities on the backend to avoid
  crasing renderer later.

Bug:  850978 ,  844377 
Change-Id: Ib4e2b444473638d896eb716fb7d9c587f5560335
Reviewed-on: https://chromium-review.googlesource.com/1096283
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#567024}
Reviewed-on: https://chromium-review.googlesource.com/1106812
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#457}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[add] https://crrev.com/e2ce4979a905dfcb953cb56ded2ddc2cbb1bdd4f/third_party/WebKit/LayoutTests/http/tests/devtools/oopif/oopif-storage-expected.txt
[add] https://crrev.com/e2ce4979a905dfcb953cb56ded2ddc2cbb1bdd4f/third_party/WebKit/LayoutTests/http/tests/devtools/oopif/oopif-storage.js
[modify] https://crrev.com/e2ce4979a905dfcb953cb56ded2ddc2cbb1bdd4f/third_party/blink/renderer/devtools/front_end/resources/ApplicationPanelSidebar.js
[modify] https://crrev.com/e2ce4979a905dfcb953cb56ded2ddc2cbb1bdd4f/third_party/blink/renderer/devtools/front_end/resources/DOMStorageModel.js
[modify] https://crrev.com/e2ce4979a905dfcb953cb56ded2ddc2cbb1bdd4f/third_party/blink/renderer/devtools/front_end/sdk/TargetManager.js
[modify] https://crrev.com/e2ce4979a905dfcb953cb56ded2ddc2cbb1bdd4f/third_party/blink/renderer/modules/modules_initializer.cc
[modify] https://crrev.com/e2ce4979a905dfcb953cb56ded2ddc2cbb1bdd4f/third_party/blink/renderer/modules/storage/inspector_dom_storage_agent.cc
[modify] https://crrev.com/e2ce4979a905dfcb953cb56ded2ddc2cbb1bdd4f/third_party/blink/renderer/modules/storage/inspector_dom_storage_agent.h

Able to reproduce the issue on chrome version 69.0.3452.0 (build without fix)
Verified the fix on Ubuntu 17.10,Windows10 and Mac 10.13.3 using Chrome version # 68.0.3440.33 as per the comment #0.
Attaching scree-cast for reference.
Observed that the "://"" entries are gone as per the fix confirmation in C#16
The fix is working as expected, adding Verified labels

Thanks...!

Deleted: 844377.mp4 1.8 MB

	844377.mp4 1.8 MB View Download