Pass blob info through from the IPC to the backing store on put.

Pass blob info through from the IPC to the backing store on put.
It's not used yet; this is all just preparatory plumbing.

BUG=108012
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=264380

[cmumford, 2014-04-10 16:59:42]

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_backing_store.cc (right):

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.cc:54:
.AddExtension(FILE_PATH_LITERAL(".indexeddb.blob"));
Nit: Consider removing leading '.' as FilePath::kExtensionSeparator is possibly
(albeit extremely unlikely) different for other platforms.

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.cc:453: for
(std::set<int>::iterator iter = child_process_ids_granted_.begin();
Nit: const_iterator?

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.cc:2816:
BlobChangeMap::iterator iter = blob_change_map_.begin();
Can you use STLDeleteContainerPairSecondPointers()?

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.cc:2850:
blob_info_.swap(*blob_info);
Why swap the blob info vs. operator=()

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_backing_store.h (right):

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.h:353: BlobChangeRecord() {}
Initialize object_store_id_. My preference would be to do away with the default
constructor and create one with key and object_store_id. That would simplify the
construction/use in Transaction::PutBlobInfo().

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.h:356: void
set_object_store_id(int64 object_store_id) {
Can possibly eliminate need for set_key() and set_object_store_id() with
constructor change? I don't know about your upcoming CL's.

[jsbell, 2014-04-11 16:19:57]

Just some initial nits.

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_backing_store.cc (right):

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.cc:54:
.AddExtension(FILE_PATH_LITERAL(".indexeddb.blob"));
On 2014/04/10 16:59:43, cmumford wrote:
> Nit: Consider removing leading '.' as FilePath::kExtensionSeparator is
possibly
> (albeit extremely unlikely) different for other platforms.

Can you give an example of where this would be problematic? We follow this
pattern elsewhere.

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_dispatcher_host.cc (right):

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_dispatcher_host.cc:122: if (message.type()
!= IndexedDBHostMsg_DatabasePut::ID)
&& instead of nested ifs?

[ericu, 2014-04-14 23:23:38]

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_backing_store.cc (right):

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.cc:54:
.AddExtension(FILE_PATH_LITERAL(".indexeddb.blob"));
On 2014/04/10 16:59:43, cmumford wrote:
> Nit: Consider removing leading '.' as FilePath::kExtensionSeparator is
possibly
> (albeit extremely unlikely) different for other platforms.

It need not be an extension separator as long as it's a legal filename
character.  It's legal on any operating system/filesystem combo we're likely to
run on.  I we hit a place where it's not, lots of other stuff in chrome
[including line 48] will blow up too.

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.cc:453: for
(std::set<int>::iterator iter = child_process_ids_granted_.begin();
On 2014/04/10 16:59:43, cmumford wrote:
> Nit: const_iterator?

Done.

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.cc:2816:
BlobChangeMap::iterator iter = blob_change_map_.begin();
On 2014/04/10 16:59:43, cmumford wrote:
> Can you use STLDeleteContainerPairSecondPointers()?

Done.  Just barely makes it shorter, what with that long function name, but I
like the change anyway for safety.

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.cc:2850:
blob_info_.swap(*blob_info);
On 2014/04/10 16:59:43, cmumford wrote:
> Why swap the blob info vs. operator=()

I believe that swapping the innards of a std::vector is constant-time, whereas a
copy would be O(N).

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_backing_store.h (right):

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.h:353: BlobChangeRecord() {}
On 2014/04/10 16:59:43, cmumford wrote:
> Initialize object_store_id_. My preference would be to do away with the
default
> constructor and create one with key and object_store_id. That would simplify
the
> construction/use in Transaction::PutBlobInfo().

Done.

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.h:356: void
set_object_store_id(int64 object_store_id) {
On 2014/04/10 16:59:43, cmumford wrote:
> Can possibly eliminate need for set_key() and set_object_store_id() with
> constructor change? I don't know about your upcoming CL's.

Done.  For future reference, the master CLs from which I'm digging these chunks
are in your review list, although they are somewhat cumbersome to grep through.

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_dispatcher_host.cc (right):

https://codereview.chromium.org/232343004/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_dispatcher_host.cc:122: if (message.type()
!= IndexedDBHostMsg_DatabasePut::ID)
On 2014/04/11 16:19:57, jsbell wrote:
> && instead of nested ifs?

Done.

[cmumford, 2014-04-15 21:14:16]

lgtm

[jsbell, 2014-04-16 20:17:10]

lgtm

[ericu, 2014-04-16 21:11:10]

The CQ bit was checked by ericu@chromium.org

[commit-bot: I haz the power, 2014-04-16 21:11:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ericu@chromium.org/232343004/100001

[commit-bot: I haz the power, 2014-04-17 01:42:34]

Change committed as 264380