IndexedDB: open-in-upgradeneeded layout test

[David Grogan]

IndexedDB: open-in-upgradeneeded layout test

[David Grogan]

Created attachment 162370 [details]
Patch

[David Grogan]

This should wait until after 90411 lands.

This currently behaves differently in DRT and content_shell, though I'm not sure which is more correct.  Suppose we call the result of this test in DRT "A" and the result from content_shell "B".  "B" is included in this patch.  It's also what firefox produces.

If the code from 90411 is patched in, the result of this test in DRT is now "B".  But then the result from content_shell ("C") is different from either "A" or "B".  "C" is definitely wrong.

So I'll wait until 90411 lands and sort this out after.

[WebKit Review Bot]

Comment on attachment 162370 [details]
Patch

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

New failing tests:
storage/indexeddb/intversion-open-in-upgradeneeded.html

[Joshua Bell]

I just tried this patch again, and it passes in DRT and content_shell's output matches.

Did we simplify 90411 enough to eliminate the "B" => "C" change?

[David Grogan]

Created attachment 165022 [details]
Patch

[David Grogan]

Yeah, seems to work.  Can you look it over for an LGTM?

[Joshua Bell]

Comment on attachment 165022 [details]
Patch

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

lgtm

> LayoutTests/storage/indexeddb/resources/intversion-open-in-upgradeneeded.js:36
> +    transaction.oncomplete = function() {

Just as a style thing, if this was written as:

transaction.oncomplete = function transactionOnComplete(evt) {

... then you could use preamble(evt); and get the name printed out while still (IMHO) keeping the readability benefits of a function expression.

(I haven't done that anywhere myself, was just pondering it while reading this patch.)

> LayoutTests/storage/indexeddb/resources/intversion-open-in-upgradeneeded.js:55
> +    evalAndLog("transaction = db.transaction('os')");

Is this just to confirm that the connection is still open? Maybe a debug() line explaining it would help.

[Joshua Bell]

Is this waiting on anything?

[David Grogan]

Created attachment 167664 [details]
Patch

[David Grogan]

Created attachment 167665 [details]
Patch

[David Grogan]

Tony, could you review this test?

[WebKit Review Bot]

Comment on attachment 167665 [details]
Patch

Clearing flags on attachment: 167665

Committed r130791: <http://trac.webkit.org/changeset/130791>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.