Open Bug 1817893 Opened 1 year ago Updated 1 year ago

Remove CacheStorage.cpp's IsTrusted helper and make CachesEnabled check honor devtools ServiceWorkersTestingEnabled browsing context flag

Categories

(Core :: Storage: Cache API, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: asuth, Unassigned)

References

Details

In bug 1112134 we introduced nsGlobalWindowInner::CachesEnabled to do a proper SecureContext check, but we did not remove the IsTrusted method which is called by the CreateOn* methods and served as an approximation of the secure context check. We should be able to remove IsTrusted now.

The one behavior that the IsTrusted control flow path handles that CachesEnabled does not, is checking the BrowsingContext ServiceWorkersTestingEnabled flag that devtools sets to allow a page to use ServiceWorkers despite being http. That check is done in nsGlobalWindowInner::GetCaches and passed into CacheStorage::CreateOnMainThread, which is of course too late since CachesEnabled already would have returned false.

It's possible we haven't seen a bug report for this because as we can see in the call graph for the getter flag the enabling check for ServiceWorkers ServiceWorkersEnabled calls IsServiceWorkersTestingEnabledInWindow which does check the browsing context. However, while this would make it look like ServiceWorkers are working in that case, and there is an effort to propagate the flag onto the WorkerLoadInfoData (see diagram of field use), the same CachesEnabled enabling function is used on workers, so any non-trivial ServiceWorker would likely fail to install.

The tl;dr for this is that CachesEnabled should be doing the same check ServiceWorkersEnabled is doing, and it's reasonable to do this as part of removing IsTrusted.

Summary: Remove CacheStorage.cpp's IsTrusted helper → Remove CacheStorage.cpp's IsTrusted helper and make CachesEnabled check honor devtools ServiceWorkersTestingEnabled browsing context flag
You need to log in before you can comment on or make changes to this bug.