Issue 1323323002: IndexedDB: Various C++11isms and cleanup

Issue 1323323002: IndexedDB: Various C++11isms and cleanup (Closed)

Created:
5 years, 3 months ago by jsbell

Modified:
5 years, 3 months ago

Reviewers:
cmumford

CC:
blink-reviews, jsbell+idb_chromium.org, dgrogan, cmumford

Base URL:
svn://svn.chromium.org/blink/trunk

Target Ref:
refs/heads/master

Project:
blink

Visibility:
Public.

More Reviews

Description

IndexedDB: Various C++11isms and cleanup * Initialize simple members in header files * Mark const members as such * Range-based iteration over containers * Remove unused typedefs * Use nullptr instead of 0 No behavior change. R=cmumford@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201850

Patch Set 1 #

Patch Set 2 : True, false, whatever #

Total comments: 6

Created: 5 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+62 lines, -90 lines)			Patch
M	Source/modules/indexeddb/IDBCursor.h	View		1 chunk	+4 lines, -4 lines	0 comments	Download
M	Source/modules/indexeddb/IDBCursor.cpp	View		1 chunk	+0 lines, -4 lines	0 comments	Download
M	Source/modules/indexeddb/IDBDatabase.h	View		1 chunk	+3 lines, -4 lines	2 comments	Download
M	Source/modules/indexeddb/IDBDatabase.cpp	View		4 chunks	+8 lines, -10 lines	0 comments	Download
M	Source/modules/indexeddb/IDBIndex.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	Source/modules/indexeddb/IDBIndex.cpp	View		1 chunk	+0 lines, -1 line	0 comments	Download
M	Source/modules/indexeddb/IDBKeyRange.h	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	Source/modules/indexeddb/IDBObjectStore.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	Source/modules/indexeddb/IDBObjectStore.cpp	View		6 chunks	+17 lines, -18 lines	0 comments	Download
M	Source/modules/indexeddb/IDBRequest.h	View	1	3 chunks	+9 lines, -9 lines	2 comments	Download
M	Source/modules/indexeddb/IDBRequest.cpp	View		1 chunk	+0 lines, -10 lines	0 comments	Download
M	Source/modules/indexeddb/IDBTransaction.h	View		2 chunks	+6 lines, -9 lines	0 comments	Download
M	Source/modules/indexeddb/IDBTransaction.cpp	View		4 chunks	+11 lines, -17 lines	2 comments	Download

Messages

Total messages: 7 (1 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

cmumford

https://codereview.chromium.org/1323323002/diff/20001/Source/modules/indexeddb/IDBDatabase.h File Source/modules/indexeddb/IDBDatabase.h (left): https://codereview.chromium.org/1323323002/diff/20001/Source/modules/indexeddb/IDBDatabase.h#oldcode145 Source/modules/indexeddb/IDBDatabase.h:145: typedef HeapHashMap<int64_t, Member<IDBTransaction>> TransactionMap; Thank you - I hate ...

5 years, 3 months ago (2015-09-03 23:21:03 UTC) #2

jsbell

5 years, 3 months ago (2015-09-03 23:29:54 UTC) #3

https://codereview.chromium.org/1323323002/diff/20001/Source/modules/indexedd...
File Source/modules/indexeddb/IDBDatabase.h (left):

https://codereview.chromium.org/1323323002/diff/20001/Source/modules/indexedd...
Source/modules/indexeddb/IDBDatabase.h:145: typedef HeapHashMap<int64_t,
Member<IDBTransaction>> TransactionMap;
On 2015/09/03 23:21:03, cmumford wrote:
> Thank you - I hate typedefs.

Check out the one for a function reference (?) in IndexedDBClient.h. I'd like to
make that a 'using' alias instead but I don't even understand how it's working.

(Some quick reading says that function references, direct uses, or
pointers-to-pointers all get treated as just function pointers, but it still
hurts my brain.)

https://codereview.chromium.org/1323323002/diff/20001/Source/modules/indexedd...
File Source/modules/indexeddb/IDBRequest.h (right):

https://codereview.chromium.org/1323323002/diff/20001/Source/modules/indexedd...
Source/modules/indexeddb/IDBRequest.h:161: Member<IDBCursor> m_pendingCursor;
On 2015/09/03 23:21:03, cmumford wrote:
> The constructor initialized this to nullptr - I'm guessing that was redundant.

Right - it was a raw pointer at one point, not needed for Member<>

https://codereview.chromium.org/1323323002/diff/20001/Source/modules/indexedd...
File Source/modules/indexeddb/IDBTransaction.cpp (right):

https://codereview.chromium.org/1323323002/diff/20001/Source/modules/indexedd...
Source/modules/indexeddb/IDBTransaction.cpp:212: for (IDBRequest* request :
m_requestList)
On 2015/09/03 23:21:03, cmumford wrote:
> I seem to remember that aborting one request can sometimes abort (or maybe
> enqueue) another. Are you sure this is safe?

Looking at what IDBRequest::abort() does, it should be safe since there are no
synchronous side effects. It can enqueue events but they are dispatched
asynchronously.

I had similar reservations, thinking about changing destruction timing, but now
that everything is GC'd the destruction all happens later anyway.

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1323323002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1323323002/20001

5 years, 3 months ago (2015-09-06 03:05:37 UTC) #6

commit-bot: I haz the power

5 years, 3 months ago (2015-09-06 04:15:41 UTC) #7

Message was sent while issue was closed.

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201850

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages