[SyncFS] Replace leveldb classes with LevelDBWrapper

[SyncFS] Replace leveldb classes with LevelDBWrapper.
In most files under sync_file_system/drive_backend/,
leveldb::DB and leveldb::WriteBatch are replaced with
LevelDBWrapper to have a database transparent.

BUG=347425
TEST=./unit_tests --gtest_filter="MetadataDatabase*:DriveBackend*:LevelDBWrapper*"

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

[peria, 2014-07-30 10:10:01]

PTL.

Most of changes are only replacing changes, but this CL has many diffs, so
reviewing part by part is also welcome.

[nhiroki, 2014-08-01 09:43:39]

Looks pretty good. Nitpicks only.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/drive_backend_util.h (right):

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/drive_backend_util.h:38:
LevelDBWrapper* db);
nit: indent is broken.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.h (right):

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.h:14: #include
"third_party/leveldatabase/src/include/leveldb/status.h"
Forward decl seems to work well.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/metadata_database.cc (right):

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database.cc:27: #include
"chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.h"
nit: You already included this in *.h

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database.cc:1060:
index_.get());
nit: can you squash this into the previous line?

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/metadata_database.h (right):

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database.h:21: #include
"chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.h"
Maybe you can remove this inclusion, or remove the forward decl at line 52.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
File
chrome/browser/sync_file_system/drive_backend/metadata_database_index_on_disk.cc
(right):

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database_index_on_disk.cc:231:
DVLOG(1) << "Adding new tracker: " << tracker->tracker_id()
It couldn't be good practice to mix non-related change with a huge patch even if
it's only a mechanical change. Can you make a separate patch for this?

(IIRC such loggers were 'DVLOG(1)' but replaced since they were noisy on the
internal page. Can you check that when you make a patch?)

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database_index_on_disk.cc:238:
DVLOG(1) << "Updating tracker: " << tracker->tracker_id()
ditto.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database_index_on_disk.cc:585:
DVLOG(1) << "  Tracker for " << tracker.file_id() << " is not an App root.";
ditto.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database_index_on_disk.cc:624:
DVLOG(1) << "  Tracker for " << tracker.file_id() << " is not an App root.";
ditto.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database_index_on_disk.cc:727:
std::string());
nit: indent

[peria, 2014-08-04 03:13:43]

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/drive_backend_util.h (right):

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/drive_backend_util.h:38:
LevelDBWrapper* db);
On 2014/08/01 09:43:38, nhiroki wrote:
> nit: indent is broken.

Done.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.h (right):

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.h:14: #include
"third_party/leveldatabase/src/include/leveldb/status.h"
On 2014/08/01 09:43:38, nhiroki wrote:
> Forward decl seems to work well.

Done.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/metadata_database.cc (right):

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database.cc:27: #include
"chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.h"
On 2014/08/01 09:43:38, nhiroki wrote:
> nit: You already included this in *.h

Acknowledged.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database.cc:1060:
index_.get());
On 2014/08/01 09:43:38, nhiroki wrote:
> nit: can you squash this into the previous line?

Done.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/metadata_database.h (right):

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database.h:21: #include
"chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.h"
On 2014/08/01 09:43:38, nhiroki wrote:
> Maybe you can remove this inclusion, or remove the forward decl at line 52.

Done.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
File
chrome/browser/sync_file_system/drive_backend/metadata_database_index_on_disk.cc
(right):

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database_index_on_disk.cc:231:
DVLOG(1) << "Adding new tracker: " << tracker->tracker_id()
On 2014/08/01 09:43:39, nhiroki wrote:
> It couldn't be good practice to mix non-related change with a huge patch even
if
> it's only a mechanical change. Can you make a separate patch for this?
> 
> (IIRC such loggers were 'DVLOG(1)' but replaced since they were noisy on the
> internal page. Can you check that when you make a patch?)

Done.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database_index_on_disk.cc:238:
DVLOG(1) << "Updating tracker: " << tracker->tracker_id()
On 2014/08/01 09:43:39, nhiroki wrote:
> ditto.

Done.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database_index_on_disk.cc:585:
DVLOG(1) << "  Tracker for " << tracker.file_id() << " is not an App root.";
On 2014/08/01 09:43:39, nhiroki wrote:
> ditto.

Done.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database_index_on_disk.cc:624:
DVLOG(1) << "  Tracker for " << tracker.file_id() << " is not an App root.";
On 2014/08/01 09:43:39, nhiroki wrote:
> ditto.

Done.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/metadata_database_index_on_disk.cc:727:
std::string());
On 2014/08/01 09:43:39, nhiroki wrote:
> nit: indent

Done.

[tzik, 2014-08-04 03:26:23]

Looks good. But this has thread race, and a transaction may be mixed to the
following.

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/drive_backend_util.h (right):

https://codereview.chromium.org/428063002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/drive_backend_util.h:38:
LevelDBWrapper* db);
indent?

https://codereview.chromium.org/428063002/diff/60001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.h (right):

https://codereview.chromium.org/428063002/diff/60001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.h:92: // usages of
driva_backend::MigrateDatabaseFromVxToVy() under
s/driva/drive/

[peria, 2014-08-04 03:33:19]

https://codereview.chromium.org/428063002/diff/60001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.h (right):

https://codereview.chromium.org/428063002/diff/60001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.h:92: // usages of
driva_backend::MigrateDatabaseFromVxToVy() under
On 2014/08/04 03:26:23, tzik wrote:
> s/driva/drive/

Done.

[tzik, 2014-08-04 06:45:40]

Could you rebase this CL onto https://codereview.chromium.org/437943002/?

[peria, 2014-08-04 06:56:56]

On 2014/08/04 06:45:40, tzik wrote:
> Could you rebase this CL onto https://codereview.chromium.org/437943002/

Done.

[tzik, 2014-08-05 06:10:55]

lgtm

[peria, 2014-08-05 06:12:08]

The CQ bit was checked by peria@chromium.org

[commit-bot: I haz the power, 2014-08-05 06:13:51]

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

[nhiroki, 2014-08-05 07:32:16]

lgtm

https://codereview.chromium.org/428063002/diff/100001/chrome/browser/sync_fil...
File chrome/browser/sync_file_system/drive_backend/drive_backend_util.cc
(right):

https://codereview.chromium.org/428063002/diff/100001/chrome/browser/sync_fil...
chrome/browser/sync_file_system/drive_backend/drive_backend_util.cc:25: if (db)
Can you replace this and others with DCHECKs in a separate CL?

[commit-bot: I haz the power, 2014-08-05 10:30:34]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-08-05 11:14:41]

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

[commit-bot: I haz the power, 2014-08-05 11:14:42]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[peria, 2014-08-05 12:05:05]

The CQ bit was checked by peria@chromium.org

[commit-bot: I haz the power, 2014-08-05 12:06:26]

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

[commit-bot: I haz the power, 2014-08-05 19:02:59]

Change committed as 287573