Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(574)

Issue 223123002: Add blob serialization support [currently unused] to SerializedScriptValue. (Closed)

Created:
6 years, 8 months ago by ericu
Modified:
6 years, 8 months ago
CC:
blink-reviews, Nils Barth (inactive), kojih, arv+blink, jsbell+bindings_chromium.org, tzik, alecflett, kouhei+bindings_chromium.org, ericu+idb_chromium.org, nhiroki, abarth-chromium, kinuko, marja+watch_chromium.org, jsbell+idb_chromium.org, dgrogan, adamk+blink_chromium.org, haraken, Nate Chapin, Inactive
Visibility:
Public.

Description

Add blob serialization support [currently unused] to SerializedScriptValue. This will be needed by IDB Blob support, in progress. BUG=108012 R=cmumford,jsbell Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=171190

Patch Set 1 #

Patch Set 2 : build fix #

Total comments: 14

Patch Set 3 : Roll in code review feedback. #

Patch Set 4 : Small fixes. #

Total comments: 2

Patch Set 5 : Update kSerializedScriptValueVersion in test. #

Total comments: 6

Patch Set 6 : Fix oilpan types #

Patch Set 7 : rebaseline crypto tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+402 lines, -275 lines) Patch
M LayoutTests/crypto/clone-aesKey-expected.txt View 1 2 3 4 5 6 48 chunks +48 lines, -48 lines 0 comments Download
M LayoutTests/crypto/clone-hmacKey-expected.txt View 1 2 3 4 5 6 48 chunks +48 lines, -48 lines 0 comments Download
M LayoutTests/crypto/clone-rsaHashedKey-private-expected.txt View 1 2 3 4 5 6 48 chunks +48 lines, -48 lines 0 comments Download
M LayoutTests/crypto/clone-rsaHashedKey-public-expected.txt View 1 2 3 4 5 6 48 chunks +48 lines, -48 lines 0 comments Download
M LayoutTests/crypto/clone-rsaKey-private-expected.txt View 1 2 3 4 5 6 16 chunks +16 lines, -16 lines 0 comments Download
M LayoutTests/crypto/clone-rsaKey-public-expected.txt View 1 2 3 4 5 6 16 chunks +16 lines, -16 lines 0 comments Download
M LayoutTests/fast/storage/resources/serialized-script-value.js View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/v8/IDBBindingUtilities.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/v8/SerializedScriptValue.h View 1 2 3 6 chunks +12 lines, -6 lines 0 comments Download
M Source/bindings/v8/SerializedScriptValue.cpp View 1 2 3 4 5 26 chunks +158 lines, -41 lines 0 comments Download
M Source/core/fileapi/File.h View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
M Source/modules/indexeddb/IDBObjectStore.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
M public/web/WebSerializedScriptValueVersion.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 24 (0 generated)
ericu
6 years, 8 months ago (2014-04-03 01:06:59 UTC) #1
cmumford
https://codereview.chromium.org/223123002/diff/20001/Source/bindings/v8/SerializedScriptValue.cpp File Source/bindings/v8/SerializedScriptValue.cpp (right): https://codereview.chromium.org/223123002/diff/20001/Source/bindings/v8/SerializedScriptValue.cpp#newcode502 Source/bindings/v8/SerializedScriptValue.cpp:502: for (unsigned i = 0; i < length; ++i) ...
6 years, 8 months ago (2014-04-03 16:32:43 UTC) #2
jsbell
https://codereview.chromium.org/223123002/diff/20001/Source/bindings/v8/SerializedScriptValue.cpp File Source/bindings/v8/SerializedScriptValue.cpp (right): https://codereview.chromium.org/223123002/diff/20001/Source/bindings/v8/SerializedScriptValue.cpp#newcode92 Source/bindings/v8/SerializedScriptValue.cpp:92: // NOTE: be sure to change wireFormatVersion as necessary! ...
6 years, 8 months ago (2014-04-03 18:16:33 UTC) #3
ericu
https://codereview.chromium.org/223123002/diff/20001/Source/bindings/v8/SerializedScriptValue.cpp File Source/bindings/v8/SerializedScriptValue.cpp (right): https://codereview.chromium.org/223123002/diff/20001/Source/bindings/v8/SerializedScriptValue.cpp#newcode92 Source/bindings/v8/SerializedScriptValue.cpp:92: // NOTE: be sure to change wireFormatVersion as necessary! ...
6 years, 8 months ago (2014-04-07 23:14:19 UTC) #4
jsbell
lgtm (will need a bindings OWNER though) https://codereview.chromium.org/223123002/diff/60001/Source/bindings/v8/SerializedScriptValue.h File Source/bindings/v8/SerializedScriptValue.h (right): https://codereview.chromium.org/223123002/diff/60001/Source/bindings/v8/SerializedScriptValue.h#newcode71 Source/bindings/v8/SerializedScriptValue.h:71: static const ...
6 years, 8 months ago (2014-04-08 16:45:01 UTC) #5
cmumford
On 2014/04/08 16:45:01, jsbell wrote: > lgtm (will need a bindings OWNER though) > > ...
6 years, 8 months ago (2014-04-08 17:08:34 UTC) #6
ericu
+abarth as blink owner. https://codereview.chromium.org/223123002/diff/60001/Source/bindings/v8/SerializedScriptValue.h File Source/bindings/v8/SerializedScriptValue.h (right): https://codereview.chromium.org/223123002/diff/60001/Source/bindings/v8/SerializedScriptValue.h#newcode71 Source/bindings/v8/SerializedScriptValue.h:71: static const uint32_t wireFormatVersion = ...
6 years, 8 months ago (2014-04-08 17:35:16 UTC) #7
abarth-chromium
public/ LGTM @jsbell: Should we make you a per-file owner of this API header?
6 years, 8 months ago (2014-04-08 21:07:00 UTC) #8
sof
https://codereview.chromium.org/223123002/diff/80001/Source/bindings/v8/SerializedScriptValue.cpp File Source/bindings/v8/SerializedScriptValue.cpp (right): https://codereview.chromium.org/223123002/diff/80001/Source/bindings/v8/SerializedScriptValue.cpp#newcode2287 Source/bindings/v8/SerializedScriptValue.cpp:2287: PassRefPtr<File> readFileHelper() Change this back to PassRefPtrWillBeRawPtr<File>. https://codereview.chromium.org/223123002/diff/80001/Source/bindings/v8/SerializedScriptValue.cpp#newcode2321 Source/bindings/v8/SerializedScriptValue.cpp:2321: ...
6 years, 8 months ago (2014-04-08 21:07:54 UTC) #9
ericu
https://codereview.chromium.org/223123002/diff/80001/Source/bindings/v8/SerializedScriptValue.cpp File Source/bindings/v8/SerializedScriptValue.cpp (right): https://codereview.chromium.org/223123002/diff/80001/Source/bindings/v8/SerializedScriptValue.cpp#newcode2287 Source/bindings/v8/SerializedScriptValue.cpp:2287: PassRefPtr<File> readFileHelper() On 2014/04/08 21:07:55, sof wrote: > Change ...
6 years, 8 months ago (2014-04-08 21:40:56 UTC) #10
ericu
Sigbjorn, I'll wait for your LGTM. On Tue, Apr 8, 2014 at 2:40 PM, <ericu@chromium.org> ...
6 years, 8 months ago (2014-04-08 21:42:18 UTC) #11
jsbell
On 2014/04/08 21:07:00, abarth wrote: > public/ LGTM > > @jsbell: Should we make you ...
6 years, 8 months ago (2014-04-09 00:29:54 UTC) #12
sof
On 2014/04/08 21:42:18, ericu wrote: > Sigbjorn, I'll wait for your LGTM. > sorry about ...
6 years, 8 months ago (2014-04-09 05:14:36 UTC) #13
ericu
The CQ bit was checked by ericu@chromium.org
6 years, 8 months ago (2014-04-09 16:11:28 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ericu@chromium.org/223123002/100001
6 years, 8 months ago (2014-04-09 16:11:33 UTC) #15
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 8 months ago (2014-04-09 16:44:52 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_rel
6 years, 8 months ago (2014-04-09 16:44:52 UTC) #17
ericu
The CQ bit was checked by ericu@chromium.org
6 years, 8 months ago (2014-04-09 17:57:12 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ericu@chromium.org/223123002/120001
6 years, 8 months ago (2014-04-09 17:57:18 UTC) #19
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 8 months ago (2014-04-09 18:57:31 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on win_blink_rel
6 years, 8 months ago (2014-04-09 18:57:31 UTC) #21
ericu
The CQ bit was checked by ericu@chromium.org
6 years, 8 months ago (2014-04-09 22:25:27 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ericu@chromium.org/223123002/120001
6 years, 8 months ago (2014-04-09 22:25:35 UTC) #23
commit-bot: I haz the power
6 years, 8 months ago (2014-04-09 22:27:28 UTC) #24
Message was sent while issue was closed.
Change committed as 171190

Powered by Google App Engine
This is Rietveld 408576698