Issue 17060008: Add dataLoss property to IDB's upgradeneeded event

Issue 17060008: Add dataLoss property to IDB's upgradeneeded event (Closed)

Created:
7 years, 6 months ago by dgrogan

Modified:
7 years, 6 months ago

Reviewers:
jsbell, dglazkov

CC:
blink-reviews, jamesr, jsbell, eae+blinkwatch, alecflett, abarth-chromium, dgrogan

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

Visibility:
Public.

More Reviews

Description

Add dataLoss property to IDB's upgradeneeded event. When the backend indicates that it performed cleanup on the backing store set the dataLoss property on the upgradeneeded event to "total". Chrome side https://codereview.chromium.org/17033004/ BUG=172626 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=152615

Patch Set 1 #

Patch Set 2 : remove debug #

Total comments: 4

Patch Set 3 : respond to comments #

Patch Set 4 : change 0 to false with comment #

Patch Set 5 : change to enum #

Patch Set 6 : fix last usages of the bool #

Total comments: 4

Patch Set 7 : events.js and add DataLoss prefix #

Patch Set 8 : Add blink API stub for compatibility with and without chromium changes #

Created: 7 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+45 lines, -14 lines)			Patch
M	LayoutTests/storage/indexeddb/data-corruption-expected.txt	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	LayoutTests/storage/indexeddb/events-expected.txt	View	1 2 3 4 5 6	1 chunk	+1 line, -0 lines	0 comments	Download
M	LayoutTests/storage/indexeddb/resources/data-corruption.js	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	LayoutTests/storage/indexeddb/resources/events.js	View	1 2 3 4 5 6	1 chunk	+1 line, -0 lines	0 comments	Download
M	Source/WebKit/chromium/src/WebIDBCallbacksImpl.h	View	1 2 3 4	1 chunk	+1 line, -1 line	0 comments	Download
M	Source/WebKit/chromium/src/WebIDBCallbacksImpl.cpp	View	1 2 3 4	1 chunk	+2 lines, -2 lines	0 comments	Download
M	Source/WebKit/chromium/tests/IDBRequestTest.cpp	View	1 2 3 4 5 6	1 chunk	+1 line, -1 line	0 comments	Download
M	Source/modules/indexeddb/IDBCallbacks.h	View	1 2 3 4	2 chunks	+2 lines, -1 line	0 comments	Download
M	Source/modules/indexeddb/IDBOpenDBRequest.h	View	1 2 3 4	1 chunk	+1 line, -1 line	0 comments	Download
M	Source/modules/indexeddb/IDBOpenDBRequest.cpp	View	1 2 3 4	2 chunks	+2 lines, -2 lines	0 comments	Download
M	Source/modules/indexeddb/IDBVersionChangeEvent.h	View	1 2 3 4 5 6	2 chunks	+5 lines, -2 lines	0 comments	Download
M	Source/modules/indexeddb/IDBVersionChangeEvent.cpp	View	1 2 3 4 5 6	2 chunks	+13 lines, -3 lines	0 comments	Download
M	Source/modules/indexeddb/IDBVersionChangeEvent.idl	View	1 2	1 chunk	+4 lines, -0 lines	0 comments	Download
M	public/platform/WebIDBCallbacks.h	View	1 2 3 4 5 6 7	2 chunks	+10 lines, -1 line	0 comments	Download

Messages

Total messages: 14 (0 generated)

Expand Messages | Collapse Messages

dgrogan

Josh, could you review this? Most of both of these patches is just plumbing.

7 years, 6 months ago (2013-06-14 20:17:19 UTC) #1

jsbell

Overall lgtm although avoiding bool arguments is preferred, especially as this may be an enum ...

7 years, 6 months ago (2013-06-14 20:44:04 UTC) #2

dgrogan

https://codereview.chromium.org/17060008/diff/1001/Source/modules/indexeddb/IDBVersionChangeEvent.cpp File Source/modules/indexeddb/IDBVersionChangeEvent.cpp (right): https://codereview.chromium.org/17060008/diff/1001/Source/modules/indexeddb/IDBVersionChangeEvent.cpp#newcode55 Source/modules/indexeddb/IDBVersionChangeEvent.cpp:55: return "total"; On 2013/06/14 20:44:04, jsbell wrote: > Should ...

7 years, 6 months ago (2013-06-14 22:41:14 UTC) #3

dgrogan

Josh, could you take another look? I declared the enum in public/platform/WebIDBCallbacks.h and used it ...

7 years, 6 months ago (2013-06-17 22:52:19 UTC) #4

jsbell

lgtm with one suggestion https://codereview.chromium.org/17060008/diff/12001/public/platform/WebIDBCallbacks.h File public/platform/WebIDBCallbacks.h (right): https://codereview.chromium.org/17060008/diff/12001/public/platform/WebIDBCallbacks.h#newcode49 public/platform/WebIDBCallbacks.h:49: None = 0, Probably want ...

7 years, 6 months ago (2013-06-17 23:16:28 UTC) #5

jsbell

https://codereview.chromium.org/17060008/diff/12001/LayoutTests/storage/indexeddb/resources/data-corruption.js File LayoutTests/storage/indexeddb/resources/data-corruption.js (right): https://codereview.chromium.org/17060008/diff/12001/LayoutTests/storage/indexeddb/resources/data-corruption.js#newcode13 LayoutTests/storage/indexeddb/resources/data-corruption.js:13: shouldBeEqualToString("event.dataLoss", "none"); You should also add a "'dataLoss' in ...

7 years, 6 months ago (2013-06-17 23:37:35 UTC) #6

dgrogan

7 years, 6 months ago (2013-06-17 23:52:14 UTC) #7

dgrogan

dglazkov@, could you review the public/ change as Blink API owner?

7 years, 6 months ago (2013-06-18 00:26:05 UTC) #8

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dgrogan@chromium.org/17060008/21001

7 years, 6 months ago (2013-06-18 02:38:58 UTC) #11

commit-bot: I haz the power

Step "update" is always a major failure. Look at the try server FAQ for more ...

7 years, 6 months ago (2013-06-18 02:49:17 UTC) #12

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dgrogan@chromium.org/17060008/21001

7 years, 6 months ago (2013-06-18 02:53:36 UTC) #13

Message was sent while issue was closed.

Change committed as 152615

Expand Messages | Collapse Messages