Summary: | Web Inspector: Storage: don't request the list of IndexedDB database names multiple times for the same security origin | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Devin Rousso <hi> | ||||
Component: | Web Inspector | Assignee: | Devin Rousso <hi> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | hi, inspector-bugzilla-changes, joepeck, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Devin Rousso
2020-05-21 22:12:37 PDT
Created attachment 400026 [details]
Patch
Comment on attachment 400026 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400026&action=review r=me > Source/WebInspectorUI/ChangeLog:11 > + This can happen if additional frames are added that share the same security origin as the > + main frame. Simply maintain a `Set` of security origins that've already been requested for > + and ignore any repeat requests. In general Storage probably doesn't respect the double key'd nature of storages of (top frame, inner frame). But I think this is probably fine since the same origin within two subframes is probably the same. Comment on attachment 400026 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400026&action=review >> Source/WebInspectorUI/ChangeLog:11 >> + and ignore any repeat requests. > > In general Storage probably doesn't respect the double key'd nature of storages of (top frame, inner frame). But I think this is probably fine since the same origin within two subframes is probably the same. We kinda do? 😅 The logic in `InspectorIndexedDBAgent::requestDatabaseNames` does use the `Document::securityOrigin` and Document::topOrigin` when getting the database names, so maybe we should also walk the `WI.Frame` ancestors and create a paired key for `_requestSecurityOrigins` instead. I was thinking that we didn't do this based on the fact that `IndexedDB.requestDatabaseNames` only took a single `securityOrigin`. Interesting that both security origins come from the same `Document` 🤔 Comment on attachment 400026 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400026&action=review >>> Source/WebInspectorUI/ChangeLog:11 >>> + and ignore any repeat requests. >> >> In general Storage probably doesn't respect the double key'd nature of storages of (top frame, inner frame). But I think this is probably fine since the same origin within two subframes is probably the same. > > We kinda do? 😅 The logic in `InspectorIndexedDBAgent::requestDatabaseNames` does use the `Document::securityOrigin` and Document::topOrigin` when getting the database names, so maybe we should also walk the `WI.Frame` ancestors and create a paired key for `_requestSecurityOrigins` instead. > > I was thinking that we didn't do this based on the fact that `IndexedDB.requestDatabaseNames` only took a single `securityOrigin`. Interesting that both security origins come from the same `Document` 🤔 Actually, i think what's here now is fine as the `Document::topOrigin` should always be the same for any given page so really the only thing that's changing is the `Document::securityOrigin`. Committed r262077: <https://trac.webkit.org/changeset/262077> All reviewed patches have been landed. Closing bug and clearing flags on attachment 400026 [details]. |