Track which IndexedDBBackingStores have been opened since boot.

Track which IndexedDBBackingStores have been opened since boot.
This way we'll know which need to have their live blob journals cleaned, once
blob support is in.

This has a small overlap with https://codereview.chromium.org/240003011/ [currently in the commit queue] for ease of merging.  However, it also fixes a bug in that CL in which GetDatabaseNames didn't have the request_context parameter.

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

[cmumford, 2014-04-29 16:08:33]

https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
File content/browser/indexed_db/indexed_db_backing_store.cc (right):

https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
content/browser/indexed_db/indexed_db_backing_store.cc:662:
net::URLRequestContext* request_context,
Why add these two unused parameters to this version of the function?

https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
File content/browser/indexed_db/indexed_db_factory.cc (right):

https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
content/browser/indexed_db/indexed_db_factory.cc:321:
scoped_refptr<IndexedDBBackingStore> IndexedDBFactory::OpenBackingStoreHelper(
I'm missing the point of this function.

https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
content/browser/indexed_db/indexed_db_factory.cc:374:
backends_opened_since_boot_.insert(origin_url);
If this is already a set why calculate/pass the first_time param? Why not just
let the set (backends_opened_since_boot_) do that by it's intrinsic nature?

[ericu, 2014-04-29 18:24:19]

https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
File content/browser/indexed_db/indexed_db_backing_store.cc (right):

https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
content/browser/indexed_db/indexed_db_backing_store.cc:662:
net::URLRequestContext* request_context,
On 2014/04/29 16:08:34, cmumford wrote:
> Why add these two unused parameters to this version of the function?

Now that I've committed blob_writer and merged out, you can see that the first
one's not unused.  The second one will signal the need to clean up the live blob
journal in a forthcoming CL.

https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
File content/browser/indexed_db/indexed_db_factory.cc (right):

https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
content/browser/indexed_db/indexed_db_factory.cc:321:
scoped_refptr<IndexedDBBackingStore> IndexedDBFactory::OpenBackingStoreHelper(
On 2014/04/29 16:08:34, cmumford wrote:
> I'm missing the point of this function.

Ah, sorry--there's no actual point in this CL.  It's needed a couple of CLs from
now to enable mocking out part of the backend for testing.  I should have
mentioned that in the CL description.

I'm putting it in this CL because 1) I can split it off from a later, larger CL;
2) I'm mucking with the parameters to all these constructors, and that makes
merging messy while I've got multiple CLs in flight at the same time.  If you
prefer, I can push this change back to the CL that needs it, but I think that's
going to be the really big one at the end.

https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
content/browser/indexed_db/indexed_db_factory.cc:374:
backends_opened_since_boot_.insert(origin_url);
On 2014/04/29 16:08:34, cmumford wrote:
> If this is already a set why calculate/pass the first_time param? Why not just
> let the set (backends_opened_since_boot_) do that by it's intrinsic nature?

I don't understand the question.  Do you mean why not just always call insert,
or why use a temporary variable instead of just passing
!backends_opened_since_boot_.count(origin_url)?  The former's a tiny
optimization [possibly unnecessary] and I think the latter makes the code a
little more readable.

[cmumford, 2014-04-29 18:45:31]

On 2014/04/29 18:24:19, ericu wrote:
>
https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
> File content/browser/indexed_db/indexed_db_backing_store.cc (right):
> 
>
https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
> content/browser/indexed_db/indexed_db_backing_store.cc:662:
> net::URLRequestContext* request_context,
> On 2014/04/29 16:08:34, cmumford wrote:
> > Why add these two unused parameters to this version of the function?
> 
> Now that I've committed blob_writer and merged out, you can see that the first
> one's not unused.  The second one will signal the need to clean up the live
blob
> journal in a forthcoming CL.
> 
>
https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
> File content/browser/indexed_db/indexed_db_factory.cc (right):
> 
>
https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
> content/browser/indexed_db/indexed_db_factory.cc:321:
> scoped_refptr<IndexedDBBackingStore> IndexedDBFactory::OpenBackingStoreHelper(
> On 2014/04/29 16:08:34, cmumford wrote:
> > I'm missing the point of this function.
> 
> Ah, sorry--there's no actual point in this CL.  It's needed a couple of CLs
from
> now to enable mocking out part of the backend for testing.  I should have
> mentioned that in the CL description.
> 
> I'm putting it in this CL because 1) I can split it off from a later, larger
CL;
> 2) I'm mucking with the parameters to all these constructors, and that makes
> merging messy while I've got multiple CLs in flight at the same time.  If you
> prefer, I can push this change back to the CL that needs it, but I think
that's
> going to be the really big one at the end.
> 
>
https://codereview.chromium.org/259063004/diff/1/content/browser/indexed_db/i...
> content/browser/indexed_db/indexed_db_factory.cc:374:
> backends_opened_since_boot_.insert(origin_url);
> On 2014/04/29 16:08:34, cmumford wrote:
> > If this is already a set why calculate/pass the first_time param? Why not
just
> > let the set (backends_opened_since_boot_) do that by it's intrinsic nature?
> 
> I don't understand the question.  Do you mean why not just always call insert,
> or why use a temporary variable instead of just passing
> !backends_opened_since_boot_.count(origin_url)?  The former's a tiny
> optimization [possibly unnecessary] and I think the latter makes the code a
> little more readable.

If I'm correct you're already doing a O(log n) lookup to set first_time so the
performance gain is very slight right? I just think that:

if (!open_in_memory)
  backends_opened_since_boot_.insert(origin_url);

and completely removing is first_time is a bit cleaner and probably just as fast
- but I do not feel strongly about this, so if you have a preference I trust
your judgement.

[cmumford, 2014-04-29 18:57:08]

lgtm

[jsbell, 2014-04-29 19:04:17]

lgtm

[ericu, 2014-04-29 19:04:38]

The CQ bit was checked by ericu@chromium.org

[commit-bot: I haz the power, 2014-04-29 19:06:20]

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

[commit-bot: I haz the power, 2014-04-29 19:47:51]

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

[commit-bot: I haz the power, 2014-04-29 19:47:51]

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

[ericu, 2014-04-29 20:06:11]

The CQ bit was checked by ericu@chromium.org

[commit-bot: I haz the power, 2014-04-29 20:33:25]

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

[commit-bot: I haz the power, 2014-04-30 01:34:24]

Change committed as 267031