<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[192803] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/192803">192803</a></dd>
<dt>Author</dt> <dd>beidson@apple.com</dd>
<dt>Date</dt> <dd>2015-11-30 10:24:25 -0800 (Mon, 30 Nov 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Modern IDB: Resolve flaky GC-vs-wrapper issue with IDBOpenDBRequest.
https://bugs.webkit.org/show_bug.cgi?id=151645
Reviewed by Andy Estes.
No new tests (Resolves flakiness with hundreds of existing IDB tests).
Do to improper management of the m_hasPendingActivity flag on IDBRequestImpl,
the request wrapper for an IDBOpenDBRequest might be garbage collected in between the
onUpgradeNeeded event and onSuccess event.
This manifested as flakiness in many tests, some more than others.
I tried to write a targeted 100% reproducible case manually forcing GC, but could not get
the timing right.
* Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp:
(WebCore::IDBClient::IDBOpenDBRequest::fireSuccessAfterVersionChangeCommit):
* Modules/indexeddb/client/IDBOpenDBRequestImpl.h:
* Modules/indexeddb/client/IDBRequestImpl.cpp:
(WebCore::IDBClient::IDBRequest::dispatchEvent):
(WebCore::IDBClient::IDBRequest::willIterateCursor):
* Modules/indexeddb/client/IDBRequestImpl.h:
(WebCore::IDBClient::IDBRequest::isOpenDBRequest):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreModulesindexeddbclientIDBOpenDBRequestImplcpp">trunk/Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp</a></li>
<li><a href="#trunkSourceWebCoreModulesindexeddbclientIDBOpenDBRequestImplh">trunk/Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.h</a></li>
<li><a href="#trunkSourceWebCoreModulesindexeddbclientIDBRequestImplcpp">trunk/Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.cpp</a></li>
<li><a href="#trunkSourceWebCoreModulesindexeddbclientIDBRequestImplh">trunk/Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (192802 => 192803)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-11-30 18:22:31 UTC (rev 192802)
+++ trunk/Source/WebCore/ChangeLog 2015-11-30 18:24:25 UTC (rev 192803)
</span><span class="lines">@@ -1,3 +1,31 @@
</span><ins>+2015-11-30 Brady Eidson <beidson@apple.com>
+
+ Modern IDB: Resolve flaky GC-vs-wrapper issue with IDBOpenDBRequest.
+ https://bugs.webkit.org/show_bug.cgi?id=151645
+
+ Reviewed by Andy Estes.
+
+ No new tests (Resolves flakiness with hundreds of existing IDB tests).
+
+ Do to improper management of the m_hasPendingActivity flag on IDBRequestImpl,
+ the request wrapper for an IDBOpenDBRequest might be garbage collected in between the
+ onUpgradeNeeded event and onSuccess event.
+
+ This manifested as flakiness in many tests, some more than others.
+
+ I tried to write a targeted 100% reproducible case manually forcing GC, but could not get
+ the timing right.
+
+ * Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp:
+ (WebCore::IDBClient::IDBOpenDBRequest::fireSuccessAfterVersionChangeCommit):
+ * Modules/indexeddb/client/IDBOpenDBRequestImpl.h:
+
+ * Modules/indexeddb/client/IDBRequestImpl.cpp:
+ (WebCore::IDBClient::IDBRequest::dispatchEvent):
+ (WebCore::IDBClient::IDBRequest::willIterateCursor):
+ * Modules/indexeddb/client/IDBRequestImpl.h:
+ (WebCore::IDBClient::IDBRequest::isOpenDBRequest):
+
</ins><span class="cx"> 2015-11-30 Per Arne Vollan <peavo@outlook.com>
</span><span class="cx">
</span><span class="cx"> [WinCairo][MediaFoundation] Implement seek.
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesindexeddbclientIDBOpenDBRequestImplcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp (192802 => 192803)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp 2015-11-30 18:22:31 UTC (rev 192802)
+++ trunk/Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp 2015-11-30 18:24:25 UTC (rev 192803)
</span><span class="lines">@@ -71,6 +71,7 @@
</span><span class="cx"> {
</span><span class="cx"> LOG(IndexedDB, "IDBOpenDBRequest::fireSuccessAfterVersionChangeCommit()");
</span><span class="cx">
</span><ins>+ ASSERT(hasPendingActivity());
</ins><span class="cx"> ASSERT(m_result);
</span><span class="cx"> ASSERT(m_result->type() == IDBAny::Type::IDBDatabase);
</span><span class="cx"> m_transaction->addRequest(*this);
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesindexeddbclientIDBOpenDBRequestImplh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.h (192802 => 192803)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.h 2015-11-30 18:22:31 UTC (rev 192802)
+++ trunk/Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.h 2015-11-30 18:24:25 UTC (rev 192803)
</span><span class="lines">@@ -61,6 +61,8 @@
</span><span class="cx"> void onUpgradeNeeded(const IDBResultData&);
</span><span class="cx"> void onDeleteDatabaseSuccess(const IDBResultData&);
</span><span class="cx">
</span><ins>+ virtual bool isOpenDBRequest() const override { return true; }
+
</ins><span class="cx"> IDBDatabaseIdentifier m_databaseIdentifier;
</span><span class="cx"> uint64_t m_version { 0 };
</span><span class="cx"> };
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesindexeddbclientIDBRequestImplcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.cpp (192802 => 192803)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.cpp 2015-11-30 18:22:31 UTC (rev 192802)
+++ trunk/Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.cpp 2015-11-30 18:24:25 UTC (rev 192803)
</span><span class="lines">@@ -238,6 +238,8 @@
</span><span class="cx"> {
</span><span class="cx"> LOG(IndexedDB, "IDBRequest::dispatchEvent - %s (%p)", event.type().characters8(), this);
</span><span class="cx">
</span><ins>+ ASSERT(m_hasPendingActivity);
+
</ins><span class="cx"> if (event.type() != eventNames().blockedEvent)
</span><span class="cx"> m_readyState = IDBRequestReadyState::Done;
</span><span class="cx">
</span><span class="lines">@@ -249,16 +251,22 @@
</span><span class="cx"> targets.append(m_transaction->db());
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ m_hasPendingActivity = false;
+
</ins><span class="cx"> bool dontPreventDefault;
</span><span class="cx"> {
</span><span class="cx"> TransactionActivator activator(m_transaction.get());
</span><span class="cx"> dontPreventDefault = IDBEventDispatcher::dispatch(event, targets);
</span><span class="cx"> }
</span><span class="cx">
</span><del>- if (m_transaction && !m_pendingCursor) {
</del><ins>+ // IDBEventDispatcher::dispatch() might have set the pending activity flag back to true, suggesting the request will be reused.
+ // We might also re-use the request if this event was the upgradeneeded event for an IDBOpenDBRequest.
+ if (!m_hasPendingActivity)
+ m_hasPendingActivity = isOpenDBRequest() && event.type() == eventNames().upgradeneededEvent;
+
+ // The request should only remain in the transaction's request list if it represents a pending cursor operation.
+ if (m_transaction && !m_pendingCursor)
</ins><span class="cx"> m_transaction->removeRequest(*this);
</span><del>- m_hasPendingActivity = false;
- }
</del><span class="cx">
</span><span class="cx"> return dontPreventDefault;
</span><span class="cx"> }
</span><span class="lines">@@ -315,6 +323,7 @@
</span><span class="cx"> ASSERT(&cursor == resultCursor());
</span><span class="cx">
</span><span class="cx"> m_pendingCursor = &cursor;
</span><ins>+ m_hasPendingActivity = true;
</ins><span class="cx"> m_result = nullptr;
</span><span class="cx"> m_readyState = IDBRequestReadyState::Pending;
</span><span class="cx"> m_domError = nullptr;
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesindexeddbclientIDBRequestImplh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.h (192802 => 192803)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.h 2015-11-30 18:22:31 UTC (rev 192802)
+++ trunk/Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.h 2015-11-30 18:24:25 UTC (rev 192803)
</span><span class="lines">@@ -113,6 +113,8 @@
</span><span class="cx"> virtual void refEventTarget() override final { RefCounted<IDBRequest>::ref(); }
</span><span class="cx"> virtual void derefEventTarget() override final { RefCounted<IDBRequest>::deref(); }
</span><span class="cx">
</span><ins>+ virtual bool isOpenDBRequest() const { return false; }
+
</ins><span class="cx"> IDBRequestReadyState m_readyState { IDBRequestReadyState::Pending };
</span><span class="cx"> RefPtr<IDBAny> m_result;
</span><span class="cx"> RefPtr<IDBTransaction> m_transaction;
</span></span></pre>
</div>
</div>
</body>
</html>