We hoped that iframes that request storage access would count the number of times the user has been asked and not repeat the request over and over. However, we're seeing pretty aggressive use of the API and users are complaining. Therefore, we should implement a cap on how many times an iframed document can ask if it is explicitly denied access by the user. This is a first measure. If we see continued aggressive use of the API, we'll have to consider more drastic measures.
<rdar://problem/51857195>
Created attachment 372576 [details] Patch
Comment on attachment 372576 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372576&action=review > Source/WebCore/dom/DocumentStorageAccess.h:69 > + void setWasExplicitlyDeniedFrameSpecificStorageAccess() { ++m_numberOfTimesExplicitlyDeniedFrameSpecificStorageAccess; }; This seems to increment count for all frames. So, if a page embedded a frame from 'social.com', a second from 'video.com', and a third from 'music.com', the 'music.com' site might get blocked even though each frame had only asked for permission once. Is that a problem? I'm trying to remember if storage access is gated on the user interacting with the frame. Are we hitting these multiple requests when just loading the page? Would it be better to perform the counting at the {document, frame} pair, instead of just all access requests of any kind incrementing a single top-level document counter?
(In reply to Brent Fulgham from comment #3) > Comment on attachment 372576 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=372576&action=review > > > Source/WebCore/dom/DocumentStorageAccess.h:69 > > + void setWasExplicitlyDeniedFrameSpecificStorageAccess() { ++m_numberOfTimesExplicitlyDeniedFrameSpecificStorageAccess; }; > > This seems to increment count for all frames. So, if a page embedded a frame > from 'social.com', a second from 'video.com', and a third from 'music.com', > the 'music.com' site might get blocked even though each frame had only asked > for permission once. I don't think that's the case. The DocumentStorageAccess class supplements Document and has a reference to the document it belongs to. So the count should be for this document in the iframe. I tried to express this scope in the bug title and explanation. > Is that a problem? > > I'm trying to remember if storage access is gated on the user interacting > with the frame. Are we hitting these multiple requests when just loading the > page? The user has to interact with the iframe. The problem is that these iframes just keep asking if the user says "Don't Allow." Being able to prompt a second time makes sense since the user might not be happy with the result of choosing "Don't Allow." But after two explicit rejections, the iframe should back off. > Would it be better to perform the counting at the {document, frame} pair, > instead of just all access requests of any kind incrementing a single > top-level document counter? See above.
Comment on attachment 372576 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372576&action=review r=me >>> Source/WebCore/dom/DocumentStorageAccess.h:69 >>> + void setWasExplicitlyDeniedFrameSpecificStorageAccess() { ++m_numberOfTimesExplicitlyDeniedFrameSpecificStorageAccess; }; >> >> This seems to increment count for all frames. So, if a page embedded a frame from 'social.com', a second from 'video.com', and a third from 'music.com', the 'music.com' site might get blocked even though each frame had only asked for permission once. >> >> Is that a problem? >> >> I'm trying to remember if storage access is gated on the user interacting with the frame. Are we hitting these multiple requests when just loading the page? >> >> Would it be better to perform the counting at the {document, frame} pair, instead of just all access requests of any kind incrementing a single top-level document counter? > > I don't think that's the case. The DocumentStorageAccess class supplements Document and has a reference to the document it belongs to. So the count should be for this document in the iframe. I tried to express this scope in the bug title and explanation. Ah! Understood. The Document here is the frame's document, not the overall top-level document. This is a good change. r=me.
(In reply to Brent Fulgham from comment #5) > Comment on attachment 372576 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=372576&action=review > > r=me > > >>> Source/WebCore/dom/DocumentStorageAccess.h:69 > >>> + void setWasExplicitlyDeniedFrameSpecificStorageAccess() { ++m_numberOfTimesExplicitlyDeniedFrameSpecificStorageAccess; }; > >> > >> This seems to increment count for all frames. So, if a page embedded a frame from 'social.com', a second from 'video.com', and a third from 'music.com', the 'music.com' site might get blocked even though each frame had only asked for permission once. > >> > >> Is that a problem? > >> > >> I'm trying to remember if storage access is gated on the user interacting with the frame. Are we hitting these multiple requests when just loading the page? > >> > >> Would it be better to perform the counting at the {document, frame} pair, instead of just all access requests of any kind incrementing a single top-level document counter? > > > > I don't think that's the case. The DocumentStorageAccess class supplements Document and has a reference to the document it belongs to. So the count should be for this document in the iframe. I tried to express this scope in the bug title and explanation. > > Ah! Understood. The Document here is the frame's document, not the overall > top-level document. > > This is a good change. r=me. I just tested to make sure, and it is scoped to the iframed document. Thanks for the review!
Comment on attachment 372576 [details] Patch Clearing flags on attachment: 372576 Committed r246647: <https://trac.webkit.org/changeset/246647>
All reviewed patches have been landed. Closing bug.