Bug 102984

Summary:

IndexedDB: Simplify transaction timers and event tracking

Product:

WebKit

Reporter:

Joshua Bell <jsbell>

Component:

New Bugs

Assignee:

Joshua Bell <jsbell>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

alecflett, dgrogan, ojan, tony, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch for landing	none

Joshua Bell

Reported 2012-11-21 15:18:33 PST

IndexedDB: Simplify transaction timers and event tracking

Attachments
Patch (16.39 KB, patch) 2012-11-21 15:28 PST, Joshua Bell	no flags	Details Formatted Diff Diff
Patch (20.08 KB, patch) 2012-11-21 16:00 PST, Joshua Bell	no flags	Details Formatted Diff Diff
Patch (20.68 KB, patch) 2012-11-27 10:42 PST, Joshua Bell	no flags	Details Formatted Diff Diff
Patch for landing (20.79 KB, patch) 2012-11-27 14:47 PST, Joshua Bell	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Joshua Bell

Comment 1 2012-11-21 15:28:38 PST

Created attachment 175536 [details] Patch

Joshua Bell

Comment 2 2012-11-21 15:30:11 PST

NOTE: http://crrev.com/11280128 needs to land first. (There some Chromium public WebKit API cleanup that can be done after this lands, as well as some cleanup inside Chromium itself.)

Joshua Bell

Comment 3 2012-11-21 15:30:54 PST

dgrogan@, alecflett@ - please take a look once Tryptophan levels have diminished to a reasonable level?

Joshua Bell

Comment 4 2012-11-21 16:00:40 PST

Created attachment 175541 [details] Patch

Joshua Bell

Comment 5 2012-11-21 16:01:33 PST

Updated patch - removed the Chromium public WebKit API impls. Just leaves an ASSERT_NOT_REACHED() impl in WebIDBTransaction.h

Joshua Bell

Comment 6 2012-11-27 09:41:51 PST

Now that turkey day has passed - alecflett@, dgrogan@ - please take a look?

Alec Flett

Comment 7 2012-11-27 10:33:19 PST

Comment on attachment 175541 [details] Patch Other than a comment request, this looks great. LGTM View in context: https://bugs.webkit.org/attachment.cgi?id=175541&action=review > Source/WebCore/Modules/indexeddb/IDBRequest.cpp:504 > if (m_transaction) { Now I'm wondering when we do and don't have an m_transaction - do you know? If so, add a comment..

Joshua Bell

Comment 8 2012-11-27 10:36:34 PST

(In reply to comment #7) > (From update of attachment 175541 [details]) > Other than a comment request, this looks great. LGTM > > View in context: https://bugs.webkit.org/attachment.cgi?id=175541&action=review > > > Source/WebCore/Modules/indexeddb/IDBRequest.cpp:504 > > if (m_transaction) { > > Now I'm wondering when we do and don't have an m_transaction - do you know? If so, add a comment.. The IDBFactory methods (open/deleteDatabase/getDatabaseNames) are the only cases. I'll add a comment.

Joshua Bell

Comment 9 2012-11-27 10:42:04 PST

Created attachment 176298 [details] Patch

Joshua Bell

Comment 10 2012-11-27 10:44:55 PST

tony@ - r?

Joshua Bell

Comment 11 2012-11-27 14:47:54 PST

Created attachment 176343 [details] Patch for landing

WebKit Review Bot

Comment 12 2012-11-27 15:09:06 PST

Comment on attachment 176343 [details] Patch for landing Clearing flags on attachment: 176343 Committed r135927: <http://trac.webkit.org/changeset/135927>

WebKit Review Bot

Comment 13 2012-11-27 15:09:10 PST

All reviewed patches have been landed. Closing bug.

Ojan Vafai

Comment 14 2012-11-30 14:29:59 PST

26% improvement: http://build.chromium.org/f/chromium/perf/chromium-rel-win7-webkit/indexeddb/report.html?rev=170079&graph=basic_insert&trace=t&history=150

Note You need to log in before you can comment on or make changes to this bug.