Bug 213418 – Storage Access API: Add the capability to call the Storage Access API as a quirk, on behalf of websites that should be doing it themselves

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[John Wilander]

There are websites and embedded content that should be calling the Storage Access API but currently are not. We should add the capability to call the API.

[Radar WebKit Bug Importer]

<rdar://problem/64549429>

[John Wilander]

Correction: We should add the capability to call the API *on their behalf*.

[John Wilander]

Created attachment 402357 [details]
Patch

[John Wilander]

Created attachment 402520 [details]
Patch

[Alex Christensen]

Comment on attachment 402520 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402520&action=review

This seems ok to me.

> Source/WebCore/dom/DocumentStorageAccess.cpp:111
> +    // The existence of a frame and page has been checked in requestStorageAccessQuickCheck().

This might be true now, but it might not be true with future use of this code.  I think it's a good idea to leave the null checks in place, maybe with assert_not_reached.

[John Wilander]

(In reply to Alex Christensen from comment #5)
> Comment on attachment 402520 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=402520&action=review
> 
> This seems ok to me.

Thanks for the review!

> > Source/WebCore/dom/DocumentStorageAccess.cpp:111
> > +    // The existence of a frame and page has been checked in requestStorageAccessQuickCheck().
> 
> This might be true now, but it might not be true with future use of this
> code.  I think it's a good idea to leave the null checks in place, maybe
> with assert_not_reached.

Right now I RELEASE_ASSERT which should force us to find the problem. Would you prefer an early return with just a debug assert?

[John Wilander]

Created attachment 402526 [details]
Patch

[John Wilander]

Waiting for EWS to make sure I fixed the Windows build issue.

[EWS]

Committed r263383: <https://trac.webkit.org/changeset/263383>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 402526 [details].

[Simon Fraser (smfr)]

Comment on attachment 402526 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402526&action=review

> Source/WebCore/dom/Element.cpp:366
> +    if (hasClass() && Quirks::StorageAccessResult::ShouldCancelEvent == document().quirks().triggerOptionalStorageAccessQuirk(eventType, classNames()))
> +        return false;

This is such a weird place to add a quirk.