Bug 108477 – Remove call to SecurityOrigin::canAccessDatabase from IDB constructor.

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

[Mike West]

We currently call SecurityOrigin::canAccessDatabase from DOMWindowIndexedDatabase::indexedDB and WorkerContextIndexedDatabase::indexedDB. I'm not sure it's necessary after http://wkbug.com/94171's patch.

Now that we're checking canAccessDatabase in the entry points to IDB, I think we can safely remove the call here. This means that the IDB object will be created, and the property will exist on the window object; it simply won't be accessible. This, I think, is more in line with the other storage mechanisms' behavior.

[Mike West]

Created attachment 185786 [details]
Patch

[Mike West]

Joshua, would you mind taking a look at this patch? I'm pretty sure these calls are now completely redundant, but you know the code much better than I.

[Joshua Bell]

Looks good to me. 

(Watch out for tests on the Chromium side that might exercise this and expect that window.indexedDB is undefined. I didn't see one in a quick scan but you may want to do a quick linux try bot run.)

[WebKit Review Bot]

Comment on attachment 185786 [details]
Patch

Attachment 185786 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16266315

New failing tests:
http/tests/security/no-indexeddb-from-sandbox.html

[Adam Barth]

Comment on attachment 185786 [details]
Patch

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

> Source/WebCore/ChangeLog:16
> +        allows us to stop holding a pointer to the ScriptExecutionContext we're
> +        extending, which can only be a good thing.

Indeed!

[Adam Barth]

> http/tests/security/no-indexeddb-from-sandbox.html

^^^ Looks like you have a test failure to work through.

[Mike West]

(In reply to comment #6)
> > http/tests/security/no-indexeddb-from-sandbox.html
> 
> ^^^ Looks like you have a test failure to work through.

Ugh. It's expecting the property not to exist in a sandboxed IFrame. I'll adjust it to expect an exception when opening the DB instead. That's consistent with the behavior when we block in a third-party context, and I think it makes more sense than hiding the API entirely.

[Mike West]

Created attachment 185959 [details]
Patch

[Build Bot]

Comment on attachment 185959 [details]
Patch

Attachment 185959 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16297527

New failing tests:
http/tests/security/no-indexeddb-from-sandbox.html

[Build Bot]

Comment on attachment 185959 [details]
Patch

Attachment 185959 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16300546

New failing tests:
http/tests/security/no-indexeddb-from-sandbox.html

[Mike West]

Created attachment 186058 [details]
Patch

[Mike West]

Amusingly, that test was "passing" on a variety of ports that don't implement IndexedDB. :) The latest patch skips it where it's not applicable.

[Adam Barth]

Comment on attachment 186058 [details]
Patch

This patch looks good.  I hopefully we won't run into compat problem from folks using the non-null-ness of window.indexedDB to check whether database access is allowed.

[WebKit Review Bot]

Comment on attachment 186058 [details]
Patch

Clearing flags on attachment: 186058

Committed r141621: <http://trac.webkit.org/changeset/141621>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.