search

electron / libchromiumcontent

Shared library build of Chromium’s Content module
MIT License
486 stars 183 forks source link

Do not check for unique origin in CacheStorage #577

Closed zcbenz closed 6 years ago

zcbenz commented 6 years ago

In Electron we may have scripts running without an origin (for example from Node.js), and it will trigger an assertion in CacheStorage. The chromium commit introduced the check is https://chromium.googlesource.com/chromium/src/+/b6a21896d9687ff8c7c6aa809e45b3efe2fefb50%5E%21/#F0.

This patch intends to disable a behavior instead of temporarily disabling a check, so I did not put it in dcheck.patch.

This should be able to fix the second crash in https://github.com/electron/electron/issues/12835#issuecomment-398941227.

deepak1556 commented 6 years ago

@zcbenz we already disable most of the storages for unique origins in content_settings_observer. There is no hook for cache storage, but it already has unique origin checks which disables all its operations https://cs.chromium.org/chromium/src/content/browser/cache_storage/cache_storage_dispatcher_host.cc?g=0&l=41 , however there are some unsolicited calls from the quota manager which doesn't have origin checks that caused this crash https://cs.chromium.org/chromium/src/content/browser/cache_storage/cache_storage_quota_client.cc. The only way to fix it with minimal patch is to disable quota manager for unique origins, since we already disable storage operations I think it should be fine disabling the manager https://cs.chromium.org/chromium/src/content/browser/storage_partition_impl.cc?g=0&l=574. Thoughts ?

zcbenz commented 6 years ago

The only way to fix it with minimal patch is to disable quota manager for unique origins, since we already disable storage operations I think it should be fine disabling the manager https://cs.chromium.org/chromium/src/content/browser/storage_partition_impl.cc?g=0&l=574. Thoughts ?

@deepak1556 It looks good to me to disable the quota manager.