Bug 102415 – IndexedDB setVersion-removal batch 7

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[David Grogan]

IndexedDB setVersion-removal batch 7

[David Grogan]

Created attachment 174495 [details]
Patch

[David Grogan]

Josh, could you look at this batch of 6-7?  After this one there are 25 tests left.

[Joshua Bell]

Comment on attachment 174495 [details]
Patch

lgtm with nits (although my eyes are glazing over)

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

> LayoutTests/storage/indexeddb/mozilla/resources/versionchange-abort.js:28
> +    trans.oncomplete = unexpectedSuccessCallback;

Should be unexpectedCompleteCallback

> LayoutTests/storage/indexeddb/resources/objectstore-basics.js:78
> +    request.onerror = addData;

addData() is only able to continue with the test because the aborted "versionchange" transaction doesn't close the database.

Can you add a FIXME referencing the existing bug on that? This test will probably need to be restructured slightly (i.e. open a new connection in addData)

> LayoutTests/storage/indexeddb/resources/shared.js:197
> +        shouldBe("openRequest.readyState", "'pending'", true/*quiet*/);

You may need to add delete self.openRequest at the end of the end of the function, otherwise the reference would "leak" and could affect tests that try and gc()? (They probably shouldn't use this method anyway, but good hygiene to not leave globals around.)

[David Grogan]

Comment on attachment 174495 [details]
Patch

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

>> LayoutTests/storage/indexeddb/mozilla/resources/versionchange-abort.js:28
>> +    trans.oncomplete = unexpectedSuccessCallback;
> 
> Should be unexpectedCompleteCallback

Fixed.

>> LayoutTests/storage/indexeddb/resources/objectstore-basics.js:78
>> +    request.onerror = addData;
> 
> addData() is only able to continue with the test because the aborted "versionchange" transaction doesn't close the database.
> 
> Can you add a FIXME referencing the existing bug on that? This test will probably need to be restructured slightly (i.e. open a new connection in addData)

FIXME added.

>> LayoutTests/storage/indexeddb/resources/shared.js:197
>> +        shouldBe("openRequest.readyState", "'pending'", true/*quiet*/);
> 
> You may need to add delete self.openRequest at the end of the end of the function, otherwise the reference would "leak" and could affect tests that try and gc()? (They probably shouldn't use this method anyway, but good hygiene to not leave globals around.)

Fixed.

[David Grogan]

Created attachment 174530 [details]
Patch

[David Grogan]

Comment on attachment 174530 [details]
Patch

Tony, could review this?

[David Grogan]

Tony, could you review this?  (I suppose it'd be helpful to actually cc you :)

[WebKit Review Bot]

Comment on attachment 174530 [details]
Patch

Clearing flags on attachment: 174530

Committed r134854: <http://trac.webkit.org/changeset/134854>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.