When an IDBTransaction is aborted, all requests that have not yet fired should fire an ABORT_ERR
Created attachment 83040 [details] Patch
please review
Comment on attachment 83040 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=83040&action=review > LayoutTests/storage/indexeddb/transaction-abort.html:55 > + request.onerror = firstAdd; Maybe set onsuccess = unexpectedSuccessFeedback? > LayoutTests/storage/indexeddb/transaction-abort.html:66 > +{ Assert that it's an ABORT_ERR?
Bugzilla won't let me submit this comment on the patch ("You are not authorized to edit attachment 83040 [details]."), but: > Source/WebCore/storage/IDBRequest.cpp:226 > + m_transaction->registerRequest(this); What does this do? It seems to be here so that if the SetVersion transaction aborts, the error handler on the request for a setversion transaction will be called. But for the transaction to abort, wouldn't the setversion success handler have to already be called? What am I missing?
(In reply to comment #3) > (From update of attachment 83040 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=83040&action=review > > > LayoutTests/storage/indexeddb/transaction-abort.html:55 > > + request.onerror = firstAdd; > > Maybe set onsuccess = unexpectedSuccessFeedback? Good idea > > LayoutTests/storage/indexeddb/transaction-abort.html:66 > > +{ > > Assert that it's an ABORT_ERR? Good idea (In reply to comment #4) > Bugzilla won't let me submit this comment on the patch ("You are not authorized to edit attachment 83040 [details]."), but: > > > Source/WebCore/storage/IDBRequest.cpp:226 > > + m_transaction->registerRequest(this); > > What does this do? > It seems to be here so that if the SetVersion transaction aborts, the error handler on the request for a setversion transaction will be called. But for the transaction to abort, wouldn't the setversion success handler have to already be called? What am I missing? Originally I asserted that if you unregistered something it had been registered before. And we don't fire onError when aborting something that already fired an onSuccess. So I believe it's OK as is, but probably a bit cleaner without. Will change.
Created attachment 83406 [details] Patch
Comment on attachment 83406 [details] Patch You have some spurious test_expectations.txt changes and conflicts that prevent EWS from running. Mind re-upping one without these?
I think the test_expectation change is a real issue...I couldn't run without it. I can't upload a new one at the moment, but I will Monday. The bots won't add much information though since this code only runs for Chromium.
Created attachment 84120 [details] Patch
Nate's OOO and this is for m11, so if someone else could look, it'd be great.
Comment on attachment 84120 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=84120&action=review > Source/WebCore/storage/IDBRequest.h:62 > + EarlyDeath = 3 Why do you use a different style for this member of the enum?
(In reply to comment #11) > (From update of attachment 84120 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=84120&action=review > > > Source/WebCore/storage/IDBRequest.h:62 > > + EarlyDeath = 3 > > Why do you use a different style for this member of the enum? Because its proper webkit style. The others match the IDL, but this isn't in IDL. I can make it match if you think that's better though. Thanks!
Comment on attachment 84120 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=84120&action=review >>> Source/WebCore/storage/IDBRequest.h:62 >>> + EarlyDeath = 3 >> >> Why do you use a different style for this member of the enum? > > Because its proper webkit style. The others match the IDL, but this isn't in IDL. I can make it match if you think that's better though. > > Thanks! OK, makes sense. So while the request is in the EarlyDeath internal-only state, what state is reported to script? Or is this not relevant/possible?
(In reply to comment #13) > (From update of attachment 84120 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=84120&action=review > > >>> Source/WebCore/storage/IDBRequest.h:62 > >>> + EarlyDeath = 3 > >> > >> Why do you use a different style for this member of the enum? > > > > Because its proper webkit style. The others match the IDL, but this isn't in IDL. I can make it match if you think that's better though. > > > > Thanks! > > OK, makes sense. So while the request is in the EarlyDeath internal-only state, what state is reported to script? Or is this not relevant/possible? Not possible. That's why readyState() has this assert: ASSERT(m_readyState == LOADING || m_readyState == DONE);
Comment on attachment 84120 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=84120&action=review >>>>> Source/WebCore/storage/IDBRequest.h:62 >>>>> + EarlyDeath = 3 >>>> >>>> Why do you use a different style for this member of the enum? >>> >>> Because its proper webkit style. The others match the IDL, but this isn't in IDL. I can make it match if you think that's better though. >>> >>> Thanks! >> >> OK, makes sense. So while the request is in the EarlyDeath internal-only state, what state is reported to script? Or is this not relevant/possible? > > Not possible. That's why readyState() has this assert: ASSERT(m_readyState == LOADING || m_readyState == DONE); I see, makes sense. Maybe add a comment in the enum that the EarlyDeath value is internal only.
Committed r80028: <http://trac.webkit.org/changeset/80028>
http://trac.webkit.org/changeset/80028 might have broken Qt Linux Release The following tests are not passing: editing/deleting/4916235-1.html editing/deleting/5390681.html editing/deleting/5546763.html editing/deleting/delete-3775172-fix.html editing/deleting/delete-at-paragraph-boundaries-007.html editing/deleting/delete-before-block-image-1.html
Caused regressions...need to fix up. http://build.webkit.org/results/Windows%207%20Release%20(Tests)/r80028%20(9911)/results.html http://build.webkit.org/results/Windows%207%20Release%20(Tests)/r80028%20(9911)/editing/deleting/6026335-crash-log.txt FAULTING_SOURCE_CODE: 100: ASSERT_UNUSED(wasAdded, wasAdded); // It should not have already been in the list. 101: 102: while (!m_queuedEvents.isEmpty()) { 103: ListHashSet<RefPtr<Event> >::iterator iter = m_queuedEvents.begin(); > 104: RefPtr<Event> event = *iter; 105: m_queuedEvents.remove(iter); 106: if (!event) 107: break; 108: dispatchEvent(event.get()); 109: }
Committed r80055: <http://trac.webkit.org/changeset/80055>