Bug 143193 – Modern IDB: ObjectStore Blob Support

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Joonghun Park]

Add IndexedDB Blob support which allows IndexedDB API to be able to store and retrieve a Blob without converting it to a Base64 string.

[Brady Eidson]

Created attachment 276501 [details]
Patch v1

This patch needs review, but won't land quite yet - https://bugs.webkit.org/show_bug.cgi?id=156640 is the last remaining blocking bug before this lands, and I'm working on it right now.

[Build Bot]

Comment on attachment 276501 [details]
Patch v1

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

New failing tests:
imported/blink/storage/indexeddb/empty-blob-file.html

[Build Bot]

Created attachment 276508 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Alex Christensen]

Comment on attachment 276501 [details]
Patch v1

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

Code changes look good.  Look into the failing test before landing, after the other patch is landed.

> LayoutTests/imported/blink/storage/indexeddb/blob-basics-metadata.html:106
> +        cursor.continue();
> +//        evalAndLog("cursor.continue();");

this doesn't look good.  I guess it exists like this in blink, though...

[Brady Eidson]

The Yosemite bot failure is super mysterious.

In my reading of the code, there's exactly one place it could come from, and that one place shouldn't ever really happen:

RefPtr<IDBRequest> IDBObjectStore::putOrAdd(JSC::ExecState& state, JSC::JSValue value, RefPtr<IDBKey> key, IndexedDB::ObjectStoreOverwriteMode overwriteMode, InlineKeyCheck inlineKeyCheck, ExceptionCodeWithMessage& ec)
{
    LOG(IndexedDB, "IDBObjectStore::putOrAdd");

    auto context = scriptExecutionContextFromExecState(&state);
    if (!context) {
        ec.code = IDBDatabaseException::UnknownError;
        return nullptr;
    }
...

[Brady Eidson]

(In reply to comment #4)
> Comment on attachment 276501 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=276501&action=review
> 
> Code changes look good.  Look into the failing test before landing, after
> the other patch is landed.

Yup, looking into it now.

> 
> > LayoutTests/imported/blink/storage/indexeddb/blob-basics-metadata.html:106
> > +        cursor.continue();
> > +//        evalAndLog("cursor.continue();");
> 
> this doesn't look good.  I guess it exists like this in blink, though...

By calling it natively, the exception is uncaught and aborts the test.
Calling with evalAndLog, the exception is caught, and then the test times out.

Prefer to leave the original in place for future diffing against the blink test.

[Brady Eidson]

Might have gone without saying, but to be explicit: Can't reproduce that failure locally.

To validate my theory on what the bot is seeing, uploading a new patch for EWS.

[Brady Eidson]

Created attachment 276512 [details]
Patch for EWS exploration

[Build Bot]

Comment on attachment 276501 [details]
Patch v1

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

New failing tests:
imported/blink/storage/indexeddb/empty-blob-file.html

[Build Bot]

Created attachment 276513 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Brady Eidson]

AHA.
stderr to the rescue

ERROR: Failed copying File contents to a Blob temporary file for storage in IndexedDB (/Volumes/Data/EWS/WebKit/LayoutTests/imported/blink/storage/indexeddb/resources/empty.txt to /var/folders/7n/s62c21h54p549lcpn5v012mc0000gn/T/DumpRenderTree-rzL3dC/BlobHd0Wr3)
/Volumes/Data/EWS/WebKit/Source/WebCore/platform/network/BlobRegistryImpl.cpp(320) : auto WebCore::BlobRegistryImpl::writeBlobsToTemporaryFiles(const Vector<WTF::String> &, std::function<void (const Vector<String> &)>)::(anonymous class)::operator()() const

[Build Bot]

Comment on attachment 276512 [details]
Patch for EWS exploration

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

New failing tests:
imported/blink/storage/indexeddb/empty-blob-file.html

[Build Bot]

Created attachment 276516 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 276512 [details]
Patch for EWS exploration

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

New failing tests:
imported/blink/storage/indexeddb/empty-blob-file.html

[Build Bot]

Created attachment 276517 [details]
Archive of layout-test-results from ews116 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Brady Eidson]

Created attachment 276518 [details]
More EWS experimentation

[Brady Eidson]

Hmmm, I'm developing a working theory here - This is an empty file, literally 0 bytes.

I wonder if the behavior of our filesystem calls are subtley different on Yosemite vs other OSXs with regards to their responses on 0 byte files.

Hopefully the logging in the patch I just uploaded will tell us this, and if so, it should be an easy fix.

[Build Bot]

Comment on attachment 276518 [details]
More EWS experimentation

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

New failing tests:
imported/blink/storage/indexeddb/empty-blob-file.html

[Build Bot]

Created attachment 276520 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 276518 [details]
More EWS experimentation

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

New failing tests:
imported/blink/storage/indexeddb/empty-blob-file.html

[Build Bot]

Created attachment 276523 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Brady Eidson]

Wow, okay.

Can't even open the source file...

One more EWS test.

[Brady Eidson]

Created attachment 276525 [details]
Patch for EWS exploration

[Build Bot]

Comment on attachment 276525 [details]
Patch for EWS exploration

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

New failing tests:
imported/blink/storage/indexeddb/empty-blob-file.html

[Build Bot]

Created attachment 276529 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Brady Eidson]

On a whim, I just tried stashing my changes and then applying the patch locally, using svn-apply.

The empty.txt file - which is in the patch - is not created on the filesystem.

So attempts to reference it do, of course, fail.

This isn't a bug with the patch, but rather a bug with svn-apply.

This patch will be fine as-is when I actually push the changed, but just cannot be svn-applied.

*sigh*

Filed https://bugs.webkit.org/show_bug.cgi?id=156649 for the svn-apply issue.

[Build Bot]

Comment on attachment 276525 [details]
Patch for EWS exploration

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

New failing tests:
imported/blink/storage/indexeddb/empty-blob-file.html

[Build Bot]

Created attachment 276534 [details]
Archive of layout-test-results from ews116 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Brady Eidson]

r156640 landed overnight.

Once I'm in the office, will re-run tests locally and land this.

[Brady Eidson]

Almost certainly lingering edge cases, but the primary functional is definitely done:
http://trac.webkit.org/changeset/199730

[Brady Eidson]

Release build fix in https://trac.webkit.org/changeset/199732