Bug 150455 - Modern IDB: Basic createObjectStore implementation
Summary: Modern IDB: Basic createObjectStore implementation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 149117
  Show dependency treegraph
 
Reported: 2015-10-22 11:03 PDT by Brady Eidson
Modified: 2015-10-24 17:24 PDT (History)
4 users (show)

See Also:


Attachments
Patch v1 (91.08 KB, patch)
2015-10-22 11:08 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch v2 (91.07 KB, patch)
2015-10-22 11:12 PDT, Brady Eidson
achristensen: commit-queue-
Details | Formatted Diff | Diff
Patch v3 (91.10 KB, patch)
2015-10-22 11:45 PDT, Brady Eidson
achristensen: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-mavericks (631.95 KB, application/zip)
2015-10-22 12:20 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews112 for mac-yosemite (764.30 KB, application/zip)
2015-10-22 12:35 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2015-10-22 11:03:38 PDT
Modern IDB: Basic createObjectStore implementation
Comment 1 Brady Eidson 2015-10-22 11:08:46 PDT
Created attachment 263837 [details]
Patch v1
Comment 2 WebKit Commit Bot 2015-10-22 11:12:18 PDT
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.
Comment 3 Brady Eidson 2015-10-22 11:12:55 PDT
Created attachment 263838 [details]
Patch v2
Comment 4 WebKit Commit Bot 2015-10-22 11:15:43 PDT
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.
Comment 5 Brady Eidson 2015-10-22 11:45:39 PDT
Created attachment 263840 [details]
Patch v3
Comment 6 WebKit Commit Bot 2015-10-22 11:48:47 PDT
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.
Comment 7 Alex Christensen 2015-10-22 11:58:45 PDT
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?
Comment 8 Brady Eidson 2015-10-22 12:05:36 PDT
(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.
Comment 9 Build Bot 2015-10-22 12:20:02 PDT
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
Comment 10 Build Bot 2015-10-22 12:20:09 PDT
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
Comment 11 Build Bot 2015-10-22 12:35:29 PDT
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
Comment 12 Build Bot 2015-10-22 12:35:33 PDT
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
Comment 13 Brady Eidson 2015-10-22 13:19:46 PDT
I've been unable to reproduce this failure locally. I'm looking in to it.
Comment 14 Brady Eidson 2015-10-22 13:31:34 PDT
(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.
Comment 15 Brady Eidson 2015-10-22 13:33:23 PDT
(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
Comment 16 Brady Eidson 2015-10-22 13:38:34 PDT
Skipped in https://trac.webkit.org/changeset/191470