Bug 150455 – Modern IDB: Basic createObjectStore implementation

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

[Brady Eidson]

Modern IDB: Basic createObjectStore implementation

[Brady Eidson]

Created attachment 263837 [details]
Patch v1

[WebKit Commit Bot]

Attachment 263837 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/client/TransactionOperation.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/TransactionOperation.h:65:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/TransactionOperation.h:80:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 3 in 42 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

Created attachment 263838 [details]
Patch v2

[WebKit Commit Bot]

Attachment 263838 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/client/TransactionOperation.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/TransactionOperation.h:65:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/TransactionOperation.h:80:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 3 in 42 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

Created attachment 263840 [details]
Patch v3

[WebKit Commit Bot]

Attachment 263840 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/client/TransactionOperation.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/TransactionOperation.h:65:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/TransactionOperation.h:80:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 3 in 42 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

Comment on attachment 263838 [details]
Patch v2

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

r=me

> LayoutTests/storage/indexeddb/modern/createobjectstore-failures.html:1
> +This test exercises the obvious ways that IDBDatabase.createObjectStore can fail.

Try calling createObjectStore outside of onupgradeneeded.

> Source/WebCore/Modules/indexeddb/client/IDBTransactionImpl.cpp:307
> +    ASSERT(resultData.type() == IDBResultType::CreateObjectStoreSuccess);

resultData is not declared.

> Source/WebCore/Modules/indexeddb/shared/IDBRequestData.cpp:54
> +        m_requestIdentifier = std::make_unique<IDBResourceIdentifier>(*other.m_requestIdentifier);

This looks strange, but it safely calls the copy constructor, right?

[Brady Eidson]

(In reply to comment #7)
> Comment on attachment 263838 [details]
> Patch v2
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=263838&action=review
> 
> r=me
> 
> > LayoutTests/storage/indexeddb/modern/createobjectstore-failures.html:1
> > +This test exercises the obvious ways that IDBDatabase.createObjectStore can fail.
> 
> Try calling createObjectStore outside of onupgradeneeded.

Done.

> > Source/WebCore/Modules/indexeddb/client/IDBTransactionImpl.cpp:307
> > +    ASSERT(resultData.type() == IDBResultType::CreateObjectStoreSuccess);
> 
> resultData is not declared.

Yah boneheaded mistake in fixing EFL, fixed in V3

> > Source/WebCore/Modules/indexeddb/shared/IDBRequestData.cpp:54
> > +        m_requestIdentifier = std::make_unique<IDBResourceIdentifier>(*other.m_requestIdentifier);
> 
> This looks strange, but it safely calls the copy constructor, right?

Correct.

[Build Bot]

Comment on attachment 263840 [details]
Patch v3

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

New failing tests:
storage/indexeddb/modern/createobjectstore-basic.html

[Build Bot]

Created attachment 263843 [details]
Archive of layout-test-results from ews101 for mac-mavericks

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

[Build Bot]

Comment on attachment 263840 [details]
Patch v3

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

New failing tests:
storage/indexeddb/modern/createobjectstore-basic.html

[Build Bot]

Created attachment 263844 [details]
Archive of layout-test-results from ews112 for mac-yosemite

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

[Brady Eidson]

I've been unable to reproduce this failure locally. I'm looking in to it.

[Brady Eidson]

(In reply to comment #13)
> I've been unable to reproduce this failure locally. I'm looking in to it.

Check that - I can reproduce, sometimes, and yes the test is inherently racey.

I'm going to skip it for now, because I know what the proper fix for it is, and that proper fix will require my next patch.

[Brady Eidson]

(In reply to comment #14)
> (In reply to comment #13)
> > I've been unable to reproduce this failure locally. I'm looking in to it.
> 
> Check that - I can reproduce, sometimes, and yes the test is inherently
> racey.
> 
> I'm going to skip it for now, because I know what the proper fix for it is,
> and that proper fix will require my next patch.

And that will be in bug https://bugs.webkit.org/show_bug.cgi?id=150468

[Brady Eidson]

Skipped in https://trac.webkit.org/changeset/191470