Bug 165000 - IndexedDB 2.0: Queue up completed requests in the client, handle them one by one
Summary: IndexedDB 2.0: Queue up completed requests in the client, handle them one by one
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: 160306 164932
  Show dependency treegraph
 
Reported: 2016-11-20 14:18 PST by Brady Eidson
Modified: 2016-11-29 08:05 PST (History)
4 users (show)

See Also:


Attachments
Patch for EWS (not quite for review) (21.63 KB, patch)
2016-11-21 23:18 PST, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (25.69 KB, patch)
2016-11-22 20:13 PST, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (30.39 KB, patch)
2016-11-28 10:29 PST, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (31.36 KB, patch)
2016-11-28 23:06 PST, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2016-11-20 14:18:39 PST
IndexedDB 2.0: Queue up completed requests in the client, handle them one by one

Note: Currently we only send one operation to the server at a time, so therefore we only ever have one reply in the client at a time.

But with the patch in https://bugs.webkit.org/show_bug.cgi?id=164932 that will change, and a subtle race is introduced in event handling on the client side.

To resolve that, we'll need this refactor.
Comment 1 Brady Eidson 2016-11-21 23:18:58 PST
Created attachment 295329 [details]
Patch for EWS (not quite for review)

Giving EWS a shot before hitting the sack, hopefully this'll be ready for review tomorrow.
Comment 2 Brady Eidson 2016-11-22 20:13:55 PST
Created attachment 295349 [details]
Patch
Comment 3 Alex Christensen 2016-11-22 23:06:19 PST
Comment on attachment 295349 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:265
> +    m_currentlyCompletingRequest = nullptr;

This seems unnecessary
Comment 4 Brady Eidson 2016-11-28 08:53:47 PST
(In reply to comment #3)
> Comment on attachment 295349 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295349&action=review
> 
> > Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:265
> > +    m_currentlyCompletingRequest = nullptr;
> 
> This seems unnecessary

It keeps an assertion valid as we walk the tight loop of things to abort.
Comment 5 Brady Eidson 2016-11-28 10:22:41 PST
The problems that the EWS bots were seeing on transaction-scheduler-6 were due to a bug in the test.

Fixing that now and giving EWS another pass.
Comment 6 Brady Eidson 2016-11-28 10:29:03 PST
Created attachment 295497 [details]
Patch
Comment 7 Brady Eidson 2016-11-28 23:06:41 PST
Created attachment 295584 [details]
Patch
Comment 8 Brady Eidson 2016-11-28 23:07:58 PST
Hammered the tests locally with and without this patch, along with a fix for the test that EWS originally had a problem with. Things seem to be good now.

Will let EWS re-run on it before I cq+
Comment 9 WebKit Commit Bot 2016-11-29 08:05:36 PST
Comment on attachment 295584 [details]
Patch

Clearing flags on attachment: 295584

Committed r209069: <http://trac.webkit.org/changeset/209069>
Comment 10 WebKit Commit Bot 2016-11-29 08:05:40 PST
All reviewed patches have been landed.  Closing bug.