Bug 137034 - Completely remove all IDB properties/constructors when it is disabled at runtime
Summary: Completely remove all IDB properties/constructors when it is disabled at runtime
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: iPhone / iPad Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-09-23 11:30 PDT by Nolan Lawson
Modified: 2015-06-08 09:50 PDT (History)
10 users (show)

See Also:


Attachments
Screenshot of the Safari 7.1 web inspector on an iOS 8 device using a UIWebView (47.56 KB, image/png)
2014-09-23 11:58 PDT, Nolan Lawson
no flags Details
Patch v1 (208.80 KB, patch)
2015-06-06 21:11 PDT, Brady Eidson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-mavericks (574.91 KB, application/zip)
2015-06-06 22:01 PDT, Build Bot
no flags Details
Patch v2 - Try to fix the mavericks wk1 problem (218.90 KB, patch)
2015-06-06 22:45 PDT, Brady Eidson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (620.39 KB, application/zip)
2015-06-06 23:41 PDT, Build Bot
no flags Details
Patch v3 - Fix the Mavericks problem. (209.64 KB, patch)
2015-06-07 09:03 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nolan Lawson 2014-09-23 11:30:44 PDT
On the UIWebView in iOS8, the `window.indexedDB` object is null and read-only, which means it's impossible to shim using something like IndexedDBShim [1].

An article just appeared in Smashing Magazine a few weeks ago with a tutorial about how to use IndexedDB with IndexedDBShim [2]. Anybody following the article will end up with non-working code in an iOS 8 app due to this bug.

There's also a relevant IndexedDBShim issue [3].

[1]: https://github.com/axemclion/IndexedDBShim
[2]: http://www.smashingmagazine.com/2014/09/02/building-simple-cross-browser-offline-todo-list-indexeddb-websql/
[3]: https://github.com/axemclion/IndexedDBShim/issues/161
Comment 1 Nolan Lawson 2014-09-23 11:57:23 PDT
It may also be problematic that IndexedDB variables like IDBCursor and IDBKeyRange are also attached to the window object. I've attached a screenshot to demonstrate.
Comment 2 Nolan Lawson 2014-09-23 11:58:32 PDT
Created attachment 238555 [details]
Screenshot of the Safari 7.1 web inspector on an iOS 8 device using a UIWebView
Comment 3 Timothy Hatcher 2014-09-23 12:05:42 PDT
IndexedDB is only supported on WKWebView and Safari. UIWebView uses legacy WebKit and it does not have IndexedDB.

Having IDBCursor and IDBKeyRange on the window when IndexedDB isn't supported is likely a bug.

Any other issues with IndexedDB in Safari or WKWebView should be filed as separate bugs.
Comment 4 Radar WebKit Bug Importer 2014-09-23 12:07:12 PDT
<rdar://problem/18429374>
Comment 5 Timothy Hatcher 2014-09-23 12:14:11 PDT
Oh, yeah. They should be undefined and replaceable. Having window.indexedDB be null and read-only makes polyfills not work.
Comment 6 Dongyuan Liu 2015-01-02 17:26:34 PST
This also affects NSWebView.
Comment 7 foxtrickdev 2015-04-28 05:45:10 PDT
Same thing in global pages of Safari extensions.
bug 19318765 @ bugreport.apple.com
Comment 8 Brady Eidson 2015-06-06 21:11:30 PDT
Created attachment 254423 [details]
Patch v1
Comment 9 Brady Eidson 2015-06-06 21:57:15 PDT
The efl-wk2 failure is not due to this patch.

The mac-wk1 failure is bewildering, as I accounted for the wk1 changes in this patch (that was the primary goal, in fact)

Will have to wait until the EWS run finishes and gives me the full results.
Comment 10 Build Bot 2015-06-06 22:01:40 PDT
Comment on attachment 254423 [details]
Patch v1

Attachment 254423 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6246540163153920

New failing tests:
js/dom/global-constructors-attributes.html
Comment 11 Build Bot 2015-06-06 22:01:45 PDT
Created attachment 254424 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 12 Brady Eidson 2015-06-06 22:37:03 PDT
mac-mavericks wins out over mac-wk1, even when running wk1 on mavericks.

I guess we need a mac-mavericks-wk1
Comment 13 Brady Eidson 2015-06-06 22:38:05 PDT
mac-mavericks *is* wk1, and mac-mavericks-wk2 would be for mavericks wk2.

Sweet.
Comment 14 Brady Eidson 2015-06-06 22:40:27 PDT
Nope nevermind, mac-mavericks wins out over both mac-wk1 and mac-wk2, so that stinks.
Comment 15 Brady Eidson 2015-06-06 22:45:20 PDT
Actually mac-wk2 might win over mac-mavericks, so let's see here... I'll just try with a patch
Comment 16 Brady Eidson 2015-06-06 22:45:52 PDT
Created attachment 254427 [details]
Patch v2 - Try to fix the mavericks wk1 problem
Comment 17 Build Bot 2015-06-06 23:41:00 PDT
Comment on attachment 254427 [details]
Patch v2 - Try to fix the mavericks wk1 problem

Attachment 254427 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5973839804628992

New failing tests:
js/dom/global-constructors-attributes.html
Comment 18 Build Bot 2015-06-06 23:41:03 PDT
Created attachment 254430 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 19 Brady Eidson 2015-06-07 08:06:59 PDT
The "flexibility" of the layout test platform directories is really getting in the way here.

Might just skip this on Mavericks.
Comment 20 Brady Eidson 2015-06-07 09:03:31 PDT
Created attachment 254437 [details]
Patch v3 - Fix the Mavericks problem.
Comment 21 Geoffrey Garen 2015-06-08 09:01:43 PDT
Comment on attachment 254437 [details]
Patch v3 - Fix the Mavericks problem.

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

r=me

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:194
> +    // So to completely disable IndexedDB at runtime we have to not generate these accessors

I would say "not autogenerate".
Comment 22 WebKit Commit Bot 2015-06-08 09:50:29 PDT
Comment on attachment 254437 [details]
Patch v3 - Fix the Mavericks problem.

Clearing flags on attachment: 254437

Committed r185322: <http://trac.webkit.org/changeset/185322>
Comment 23 WebKit Commit Bot 2015-06-08 09:50:34 PDT
All reviewed patches have been landed.  Closing bug.