Migrate IDBDatabaseBackendTest from blink to chromium

Migrate IDBDatabaseBackendTest from blink to chromium

This is a fairly direct port, except for the removal of the frontend
test for ForceClosed

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

[jsbell, 2013-05-28 19:27:32]

Holding off on l-g-t-m since I think we should verify the backing store lifetime
as in the original tests.

https://codereview.chromium.org/15724006/diff/1/content/browser/indexed_db/in...
File content/browser/indexed_db/indexed_db_database_unittest.cc (right):

https://codereview.chromium.org/15724006/diff/1/content/browser/indexed_db/in...
content/browser/indexed_db/indexed_db_database_unittest.cc:25: using namespace
content;
"using namespace" is not permitted. 

Wrap the test with namespace content { ... }

https://codereview.chromium.org/15724006/diff/1/content/browser/indexed_db/in...
content/browser/indexed_db/indexed_db_database_unittest.cc:34: new
IndexedDBFakeBackingStore();
ISTM that we need a Mock backing store that has observable destruction, e.g. 

class MockBackingStore : public FakeBackingStore {
public:
  MockBackingStore(bool* alive) : alive_(alive) { *alive_ = true; }
protected:
  virtual ~MockBackingStore() { *alive_ = false; }
private:
  bool* alive_;
};

bool backing_store_alive;
scoped_refptr<MockBackingStore> backing_store =
MockBackingStore(&backing_store_alive_);
ASSERT_TRUE(backing_store_alive_);

... and then release the local scoped_refptr *earlier* than in the current tests
(i.e. as soon as it is handed off) and EXPECT(!backing_store_alive) after we
expect the last ref to have been dropped, instead of the HasOneRef() calls.

https://codereview.chromium.org/15724006/diff/1/content/browser/indexed_db/in...
content/browser/indexed_db/indexed_db_database_unittest.cc:125: request1,
connection1, 1, IndexedDBDatabaseMetadata::DEFAULT_INT_VERSION);
Make a local const int64 for the upgrade transaction_id ?

https://codereview.chromium.org/15724006/diff/1/content/browser/indexed_db/in...
content/browser/indexed_db/indexed_db_database_unittest.cc:131: request2,
connection2, 2, IndexedDBDatabaseMetadata::DEFAULT_INT_VERSION);
Ditto?

https://codereview.chromium.org/15724006/diff/1/content/browser/indexed_db/in...
content/browser/indexed_db/indexed_db_database_unittest.cc:145: virtual void
OnVersionChange(int64_t old_version, int64_t new_version)
int64_t -> int64 here and elsewhere. :(

https://codereview.chromium.org/15724006/diff/1/content/browser/indexed_db/in...
File content/browser/indexed_db/indexed_db_fake_backing_store.h (right):

https://codereview.chromium.org/15724006/diff/1/content/browser/indexed_db/in...
content/browser/indexed_db/indexed_db_fake_backing_store.h:1: // Copyright (c)
2013 The Chromium Authors. All rights reserved.
Naming convention in chromium is fake_XXXX.h/.cc - maybe we want
fake_indexed_db_backing_store.h/.cc ?

(Or maybe we should drop "indexed_db_" from all the filenames...)

[alecflett, 2013-05-28 23:07:46]

Ok issues addreessed. I tried to clean up the expectations around when the
backing store is going away, but I not be covering everything...

https://codereview.chromium.org/15724006/diff/1/content/browser/indexed_db/in...
File content/browser/indexed_db/indexed_db_database_unittest.cc (right):

https://codereview.chromium.org/15724006/diff/1/content/browser/indexed_db/in...
content/browser/indexed_db/indexed_db_database_unittest.cc:25: using namespace
content;
On 2013/05/28 19:27:32, jsbell wrote:
> "using namespace" is not permitted. 
> 
> Wrap the test with namespace content { ... }

Done.

https://codereview.chromium.org/15724006/diff/1/content/browser/indexed_db/in...
content/browser/indexed_db/indexed_db_database_unittest.cc:125: request1,
connection1, 1, IndexedDBDatabaseMetadata::DEFAULT_INT_VERSION);
On 2013/05/28 19:27:32, jsbell wrote:
> Make a local const int64 for the upgrade transaction_id ?

Done.

https://codereview.chromium.org/15724006/diff/1/content/browser/indexed_db/in...
content/browser/indexed_db/indexed_db_database_unittest.cc:131: request2,
connection2, 2, IndexedDBDatabaseMetadata::DEFAULT_INT_VERSION);
On 2013/05/28 19:27:32, jsbell wrote:
> Ditto?

Done.

https://codereview.chromium.org/15724006/diff/1/content/browser/indexed_db/in...
content/browser/indexed_db/indexed_db_database_unittest.cc:145: virtual void
OnVersionChange(int64_t old_version, int64_t new_version)
On 2013/05/28 19:27:32, jsbell wrote:
> int64_t -> int64 here and elsewhere. :(

Done.

[jsbell, 2013-05-28 23:33:15]

lgtm, but I said in person: "I'm an idiot" - as you point out, using HasOneRef()
we can implement this without the destruction tracking. So this can be
simplified...

https://codereview.chromium.org/15724006/diff/5001/content/browser/indexed_db...
File content/browser/indexed_db/indexed_db_database_unittest.cc (right):

https://codereview.chromium.org/15724006/diff/5001/content/browser/indexed_db...
content/browser/indexed_db/indexed_db_database_unittest.cc:56: backing_store =
NULL;
In hindsight, can keep backing_store around...

https://codereview.chromium.org/15724006/diff/5001/content/browser/indexed_db...
content/browser/indexed_db/indexed_db_database_unittest.cc:57:
EXPECT_TRUE(backing_store_alive);
This can be:
EXPECT_FALSE(backing_store->HasOneRef());  // local and db

https://codereview.chromium.org/15724006/diff/5001/content/browser/indexed_db...
content/browser/indexed_db/indexed_db_database_unittest.cc:59:
EXPECT_FALSE(backing_store_alive);
EXPECT_TRUE(backing_store->HasOneRef());  // local

https://codereview.chromium.org/15724006/diff/5001/content/browser/indexed_db...
content/browser/indexed_db/indexed_db_database_unittest.cc:120:
EXPECT_TRUE(backing_store_alive);
EXPECT_TRUE(backing_store->HasOneRef());  // local

https://codereview.chromium.org/15724006/diff/5001/content/browser/indexed_db...
content/browser/indexed_db/indexed_db_database_unittest.cc:128:
EXPECT_TRUE(backing_store_alive);
EXPECT_FALSE(backing_store->HasOneRef());  // local and db

https://codereview.chromium.org/15724006/diff/5001/content/browser/indexed_db...
content/browser/indexed_db/indexed_db_database_unittest.cc:131: backing_store =
NULL;
Remove

https://codereview.chromium.org/15724006/diff/5001/content/browser/indexed_db...
content/browser/indexed_db/indexed_db_database_unittest.cc:152:
EXPECT_TRUE(backing_store_alive);
EXPECT_FALSE(backing_store->HasOneRef());  // local and connection

https://codereview.chromium.org/15724006/diff/5001/content/browser/indexed_db...
content/browser/indexed_db/indexed_db_database_unittest.cc:155:
EXPECT_FALSE(backing_store_alive);
EXPECT_TRUE(backing_store->HasOneRef());  // local

https://codereview.chromium.org/15724006/diff/5001/content/browser/indexed_db...
content/browser/indexed_db/indexed_db_database_unittest.cc:207:
EXPECT_TRUE(backing_store_alive);
EXPECT_TRUE(backing_store->HasOneRef());  // local

https://codereview.chromium.org/15724006/diff/5001/content/browser/indexed_db...
content/browser/indexed_db/indexed_db_database_unittest.cc:215: backing_store =
NULL;
Remove

https://codereview.chromium.org/15724006/diff/5001/content/browser/indexed_db...
content/browser/indexed_db/indexed_db_database_unittest.cc:216:
EXPECT_TRUE(backing_store_alive);
EXPECT_FALSE(backing_store->HasOneRef());  // local and backend

https://codereview.chromium.org/15724006/diff/5001/content/browser/indexed_db...
content/browser/indexed_db/indexed_db_database_unittest.cc:239:
EXPECT_FALSE(backing_store_alive);
EXPECT_TRUE(backing_store->HasOneRef());  // local

[alecflett, 2013-05-28 23:52:42]

ok, back to HasOneRef checks

[jsbell, 2013-05-29 00:03:54]

lgtm but with a note about a comment (which is probably my fault)

https://codereview.chromium.org/15724006/diff/11001/content/browser/indexed_d...
File content/browser/indexed_db/indexed_db_database_unittest.cc (right):

https://codereview.chromium.org/15724006/diff/11001/content/browser/indexed_d...
content/browser/indexed_db/indexed_db_database_unittest.cc:125:
EXPECT_FALSE(backing_store->HasOneRef());  // local and connection
technically, db has the reference; db it just retaining it because of the
connection count.

https://codereview.chromium.org/15724006/diff/11001/content/browser/indexed_d...
content/browser/indexed_db/indexed_db_database_unittest.cc:143:
EXPECT_TRUE(backing_store->HasOneRef());
e.g. here you could ask db for it's backing store and it would have just
transitioned to null

[commit-bot: I haz the power, 2013-05-31 22:16:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alecflett@chromium.org/15724006/15001

[alecflett, 2013-05-31 22:18:01]

https://codereview.chromium.org/15724006/diff/11001/content/browser/indexed_d...
File content/browser/indexed_db/indexed_db_database_unittest.cc (right):

https://codereview.chromium.org/15724006/diff/11001/content/browser/indexed_d...
content/browser/indexed_db/indexed_db_database_unittest.cc:125:
EXPECT_FALSE(backing_store->HasOneRef());  // local and connection
On 2013/05/29 00:03:54, jsbell wrote:
> technically, db has the reference; db it just retaining it because of the
> connection count.

Done.

https://codereview.chromium.org/15724006/diff/11001/content/browser/indexed_d...
content/browser/indexed_db/indexed_db_database_unittest.cc:143:
EXPECT_TRUE(backing_store->HasOneRef());
On 2013/05/29 00:03:54, jsbell wrote:
> e.g. here you could ask db for it's backing store and it would have just
> transitioned to null

I'll do that too

[commit-bot: I haz the power, 2013-05-31 22:27:22]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[jsbell, 2013-05-31 22:29:15]

piman@ - can you OWNERS-review the gypi change?

[piman, 2013-05-31 22:31:55]

lgtm

https://chromiumcodereview.appspot.com/15724006/diff/15001/content/browser/in...
File content/browser/indexed_db/indexed_db_fake_backing_store.h (right):

https://chromiumcodereview.appspot.com/15724006/diff/15001/content/browser/in...
content/browser/indexed_db/indexed_db_fake_backing_store.h:59: OVERRIDE;
nit: odd style is odd.
We usually wrap the parameter list and leave OVERRIDE on the same line as the )

https://chromiumcodereview.appspot.com/15724006/diff/15001/content/browser/in...
content/browser/indexed_db/indexed_db_fake_backing_store.h:111: OVERRIDE;
ditto

[jsbell, 2013-05-31 22:36:11]

https://chromiumcodereview.appspot.com/15724006/diff/15001/content/browser/in...
File content/browser/indexed_db/indexed_db_fake_backing_store.h (right):

https://chromiumcodereview.appspot.com/15724006/diff/15001/content/browser/in...
content/browser/indexed_db/indexed_db_fake_backing_store.h:59: OVERRIDE;
On 2013/05/31 22:31:55, piman wrote:
> nit: odd style is odd.
> We usually wrap the parameter list and leave OVERRIDE on the same line as the
)

Probably clang-format. I'll send them a bug.

[commit-bot: I haz the power, 2013-05-31 23:30:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alecflett@chromium.org/15724006/15001

[commit-bot: I haz the power, 2013-06-01 01:03:49]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-06-02 18:44:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alecflett@chromium.org/15724006/15001

[commit-bot: I haz the power, 2013-06-02 19:18:57]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win7_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-06-03 17:42:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alecflett@chromium.org/15724006/46001

[commit-bot: I haz the power, 2013-06-03 18:45:59]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-06-03 19:29:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alecflett@chromium.org/15724006/46004

[commit-bot: I haz the power, 2013-06-03 20:44:18]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-06-03 21:32:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alecflett@chromium.org/15724006/44017

[commit-bot: I haz the power, 2013-06-04 07:33:31]

Change committed as 203897