Add the leveldb coding changes for IDB blob support.

Add the leveldb coding changes for IDB blob support.

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

[ericu, 2014-03-12 20:41:58]

And so it begins...
Here's CL #1 for the big change.

[cmumford, 2014-03-13 17:53:40]

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_leveldb_coding.cc (right):

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_leveldb_coding.cc:50: //     If the
blobKey is AllBlobsKey, the whole database should be deleted.
kAllBlobsKey?

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_leveldb_coding.cc:1858:
DCHECK_GT(encoded_user_key_.size(), 0UL);
Why not 4UL?

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_leveldb_coding.cc:1878: return ret;
Why not just?

return prefix.Encode() + encoded_user_key;

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_leveldb_coding.h (right):

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_leveldb_coding.h:234: static const int64
kInvalidBlobKey = -1;  // All keys <= 0 are invalid.
kAllBlobsKey/kInvalidBlobKey aren't used anywhere. Assuming they will be in a
future CL?

[ericu, 2014-03-13 19:25:44]

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_leveldb_coding.cc (right):

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_leveldb_coding.cc:50: //     If the
blobKey is AllBlobsKey, the whole database should be deleted.
On 2014/03/13 17:53:41, cmumford wrote:
> kAllBlobsKey?

Done.

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_leveldb_coding.cc:1858:
DCHECK_GT(encoded_user_key_.size(), 0UL);
On 2014/03/13 17:53:41, cmumford wrote:
> Why not 4UL?

That's not the lower limit of key length.  Empty arrays and empty strings are
valid keys...they appear to be encoded as a type byte and a var int [length]. 
Var int seems to have a minimum length of 1 byte, so 2 bytes would be the
minimum encoded key length.  Still, I think > 0 is clearer than > 1, since it
doesn't require the reader to go check what the minimum encoded key length is,
and a length-1 key is a pretty unusual bug.

I could just check !encoded_user_key_.empty(); perhaps that would be clearer.

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_leveldb_coding.cc:1878: return ret;
On 2014/03/13 17:53:41, cmumford wrote:
> Why not just?
> 
> return prefix.Encode() + encoded_user_key;

Why indeed?

I think this is leftover from when things used to be held in webkit data types
that didn't allow such easy manipulation.  Fixed.

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_leveldb_coding.h (right):

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_leveldb_coding.h:234: static const int64
kInvalidBlobKey = -1;  // All keys <= 0 are invalid.
On 2014/03/13 17:53:41, cmumford wrote:
> kAllBlobsKey/kInvalidBlobKey aren't used anywhere. Assuming they will be in a
> future CL?

Yup.  My trick for this sort of thing: go to the master CL, find the link for
the whole patch as a single file, and then search.

[jsbell, 2014-03-13 20:58:47]

lgtm if cmumford doesn't have anything else

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_leveldb_coding.cc (right):

https://codereview.chromium.org/198133002/diff/40001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_leveldb_coding.cc:1878: return ret;
On 2014/03/13 19:25:45, ericu wrote:
> On 2014/03/13 17:53:41, cmumford wrote:
> > Why not just?
> > 
> > return prefix.Encode() + encoded_user_key;
> 
> Why indeed?
> 
> I think this is leftover from when things used to be held in webkit data types
> that didn't allow such easy manipulation.  Fixed.

Yep. These were originally Vector<char> then std::vector<char> before finally
being std::string. I think most of these have at least been updated from
ret.insert() to ret.append(), but the master blob patch predates that.

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

https://codereview.chromium.org/198133002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_leveldb_coding.cc:1836: if (!Decode(slice,
&key))
Maybe stick in a TODO that we could be smarter and avoid a full decode/encode
since the suffix is the same in both.

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

https://codereview.chromium.org/198133002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_leveldb_coding.h:426: private:
DISALLOW_COPY_AND_ASSIGN ?

[ericu, 2014-03-13 21:05:53]

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

https://codereview.chromium.org/198133002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_leveldb_coding.cc:1836: if (!Decode(slice,
&key))
On 2014/03/13 20:58:48, jsbell wrote:
> Maybe stick in a TODO that we could be smarter and avoid a full decode/encode
> since the suffix is the same in both. 

Done.

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

https://codereview.chromium.org/198133002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_leveldb_coding.h:426: private:
On 2014/03/13 20:58:48, jsbell wrote:
> DISALLOW_COPY_AND_ASSIGN ?

Done.

[ericu, 2014-03-13 21:06:27]

The CQ bit was checked by ericu@chromium.org

[commit-bot: I haz the power, 2014-03-13 21:07:05]

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

[commit-bot: I haz the power, 2014-03-13 21:26:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-13 21:26:29]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[ericu, 2014-03-13 23:21:25]

The CQ bit was checked by ericu@chromium.org

[commit-bot: I haz the power, 2014-03-13 23:22:42]

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

[commit-bot: I haz the power, 2014-03-14 00:25:04]

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

[commit-bot: I haz the power, 2014-03-14 06:37:42]

Change committed as 257044