New issue
Advanced search Search tips

Issue 870813 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 1
Type: Feature

Blocked on:
issue 870806



Sign in to add a comment

Use a single unified proto LevelDB

Project Member Reported by nyquist@chromium.org, Aug 3

Issue description

Chrome Version: M69
OS: All

Today Chrome has ~10 unique LevelDB databases storing protocol buffers using the //components/leveldb_proto component. This is technically unnecessary, and we should investigate whether it has a positive performance impact to have a single database for browser features, using a prefix to shard the data into unique namespaces.

We should aim to keep a similar API as the current leveldb_proto::ProtoDatabase, and migrate all current clients to the new database.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c67cfe86a16a2c174ab32af869e3b434ccc93400

commit c67cfe86a16a2c174ab32af869e3b434ccc93400
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Mon Aug 13 18:14:40 2018

Add and record a histogram for LevelDB memory use after init.

Adds LevelDB.ApproximateMemoryUse.<clientname> histograms for memory
use after initialization. This should give us the size of the MemTable
right after initialization.

BUG= 870806 ,870813

Change-Id: I2c6ea6f176e92c319718ed29b6fb9192792bf7f1
Reviewed-on: https://chromium-review.googlesource.com/1162357
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582641}
[modify] https://crrev.com/c67cfe86a16a2c174ab32af869e3b434ccc93400/components/leveldb_proto/leveldb_database.cc
[modify] https://crrev.com/c67cfe86a16a2c174ab32af869e3b434ccc93400/components/leveldb_proto/leveldb_database.h
[modify] https://crrev.com/c67cfe86a16a2c174ab32af869e3b434ccc93400/tools/metrics/histograms/histograms.xml

Labels: -Target-70 Target-71
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f44e333f7fefe71b1e99fb7761988f61705372b9

commit f44e333f7fefe71b1e99fb7761988f61705372b9
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Wed Aug 22 18:11:42 2018

Add a LevelDB::Init that returns leveldb::Status.

Currently, LevelDB::Init only returns a bool and attempts to
automatically destroy corrupted databases. In preparation for a shared
database where automatic destruction isn't ideal, a new LevelDB::Init
function is added that takes a boolean |destroy_on_corruption| to allow
for the current behaviour, while also returning a leveldb::Status so
callers can determine their desired course of action in the event of
a corruption, or any other type of error.

The original Init function's behaviour remains intact so current
calls will be unaffected.

Bug: 870813
Change-Id: Id9ca0c2937abd8ed9d3dea5c218b9b229a532057
Reviewed-on: https://chromium-review.googlesource.com/1171082
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585167}
[modify] https://crrev.com/f44e333f7fefe71b1e99fb7761988f61705372b9/components/leveldb_proto/leveldb_database.cc
[modify] https://crrev.com/f44e333f7fefe71b1e99fb7761988f61705372b9/components/leveldb_proto/leveldb_database.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/43f932e3ab539702e2c668a5344b7403884b88fd

commit 43f932e3ab539702e2c668a5344b7403884b88fd
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Wed Aug 22 21:23:43 2018

Give ProtoDatabase access to Level DB's approximate memory usage.

This pipes the call to GetProperty from ProtoDatabase through to
leveldb_proto LevelDB and finally onto the underlying Level DB, so we
can retrieve the value for tests and metrics.

Bug: 870813
Change-Id: I23f89685017eeae77e19c37847d6399cc8d6085f
Reviewed-on: https://chromium-review.googlesource.com/1169878
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585252}
[modify] https://crrev.com/43f932e3ab539702e2c668a5344b7403884b88fd/components/leveldb_proto/leveldb_database.cc
[modify] https://crrev.com/43f932e3ab539702e2c668a5344b7403884b88fd/components/leveldb_proto/leveldb_database.h
[modify] https://crrev.com/43f932e3ab539702e2c668a5344b7403884b88fd/components/leveldb_proto/proto_database_impl.h

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/952e31b21b483012b2349d24a75b12f9c77e0d0e

commit 952e31b21b483012b2349d24a75b12f9c77e0d0e
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Thu Aug 23 15:51:13 2018

Add additional filtering options to LevelDB/ProtoDatabase LoadEntries.

In preparation for a single shared database for browser features,
additional parameters have been added to LoadWithFilter to allow
only specific entries to be loaded instead of the entire database.
Some minor modifications to FakeDB are made to ensure tests still
build and run.

Bug: 870813
Change-Id: I802331d477ec0ed2d6821c6d937cb2d497560345
Reviewed-on: https://chromium-review.googlesource.com/1169874
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585497}
[modify] https://crrev.com/952e31b21b483012b2349d24a75b12f9c77e0d0e/components/leveldb_proto/leveldb_database.cc
[modify] https://crrev.com/952e31b21b483012b2349d24a75b12f9c77e0d0e/components/leveldb_proto/leveldb_database.h
[modify] https://crrev.com/952e31b21b483012b2349d24a75b12f9c77e0d0e/components/leveldb_proto/proto_database.h
[modify] https://crrev.com/952e31b21b483012b2349d24a75b12f9c77e0d0e/components/leveldb_proto/proto_database_impl.h
[modify] https://crrev.com/952e31b21b483012b2349d24a75b12f9c77e0d0e/components/leveldb_proto/proto_database_impl_unittest.cc
[modify] https://crrev.com/952e31b21b483012b2349d24a75b12f9c77e0d0e/components/leveldb_proto/testing/fake_db.h

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f80c041db4bcfb986d5220362d032a7d3ae7bec1

commit f80c041db4bcfb986d5220362d032a7d3ae7bec1
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Fri Aug 24 17:20:49 2018

Add ProtoDB perf tests.

Introduces some performance tests for ProtoDatabase, for writing
individual entries, writing in batches, and writing/reading a single
database containing all entries vs. separate databases. Reports time
taken and memory used where appropriate.

Bug:870813

Change-Id: I9c916f296afed63be4d414f4a2fe70541863d510
Reviewed-on: https://chromium-review.googlesource.com/1148889
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585889}
[modify] https://crrev.com/f80c041db4bcfb986d5220362d032a7d3ae7bec1/components/BUILD.gn
[add] https://crrev.com/f80c041db4bcfb986d5220362d032a7d3ae7bec1/components/leveldb_proto/proto_database_perftest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f802f9338c8a1c8dd115db21feae55eec77be553

commit f802f9338c8a1c8dd115db21feae55eec77be553
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Mon Sep 17 18:12:19 2018

Fix LevelDB approximate memory use histogram calculation.

Subtracts the share block cache charge from the approximate memory use
we receive from the LevelDB.

Bug:  870806 ,870813
Change-Id: I90f22d6ad9c7acc29746618bde77386ab3741e17
Reviewed-on: https://chromium-review.googlesource.com/1180271
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591740}
[modify] https://crrev.com/f802f9338c8a1c8dd115db21feae55eec77be553/components/leveldb_proto/leveldb_database.cc
[modify] https://crrev.com/f802f9338c8a1c8dd115db21feae55eec77be553/components/leveldb_proto/leveldb_database.h
[modify] https://crrev.com/f802f9338c8a1c8dd115db21feae55eec77be553/tools/metrics/histograms/histograms.xml

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 24

Labels: merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/382c65b67da8ef30805792f74aa9f82d5acacbd2

commit 382c65b67da8ef30805792f74aa9f82d5acacbd2
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Mon Sep 24 17:34:27 2018

Fix LevelDB approximate memory use histogram calculation.

Subtracts the share block cache charge from the approximate memory use
we receive from the LevelDB.

Bug:  870806 ,870813
Change-Id: I90f22d6ad9c7acc29746618bde77386ab3741e17
Reviewed-on: https://chromium-review.googlesource.com/1180271
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#591740}(cherry picked from commit f802f9338c8a1c8dd115db21feae55eec77be553)
Reviewed-on: https://chromium-review.googlesource.com/1240198
Reviewed-by: Troy Hildebrandt <thildebr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#590}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/382c65b67da8ef30805792f74aa9f82d5acacbd2/components/leveldb_proto/leveldb_database.cc
[modify] https://crrev.com/382c65b67da8ef30805792f74aa9f82d5acacbd2/components/leveldb_proto/leveldb_database.h
[modify] https://crrev.com/382c65b67da8ef30805792f74aa9f82d5acacbd2/tools/metrics/histograms/histograms.xml

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c7f0b95da92da5facfe659862224c22eaea8d9c4

commit c7f0b95da92da5facfe659862224c22eaea8d9c4
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Thu Oct 11 15:53:22 2018

Refactor ProtoDatabase(Impl) to allow DBs with multiple proto types.

Currently, ProtoDatabase<T> only allows us to have a LevelDB that stores
a single proto type.

In preparation for a single unified proto DB, ProtoDatabase and
ProtoDatabaseImpl have been refactored. Much of ProtoDatabaseImpl's
logic has been moved into ProtoLevelDBWrapper, which contains template
functions instead of being a template class.

ProtoDatabase<T> provides the interface we have today, and we now
have a UniqueProtoDatabase<T> as a thin layer above ProtoDatabase so
that it can manage the unique_ptr to its own LevelDB.
ProtoDatabaseImpl<T> is kept around as an alias for UniqueProtoDatabase
for compatibility, and as a result no ProtoDatabase(Impl) users are
negatively impacted by this change.

The addition of the ProtoLevelDBWrapper provides a convenient place to
record metrics for the various database clients.

Bug: 870813
Change-Id: I6175a3cbea5dd312f09c1d88d5ad80f1f4b26006
Reviewed-on: https://chromium-review.googlesource.com/c/1170093
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598782}
[modify] https://crrev.com/c7f0b95da92da5facfe659862224c22eaea8d9c4/components/leveldb_proto/BUILD.gn
[modify] https://crrev.com/c7f0b95da92da5facfe659862224c22eaea8d9c4/components/leveldb_proto/leveldb_database.h
[modify] https://crrev.com/c7f0b95da92da5facfe659862224c22eaea8d9c4/components/leveldb_proto/proto_database.h
[modify] https://crrev.com/c7f0b95da92da5facfe659862224c22eaea8d9c4/components/leveldb_proto/proto_database_impl.h
[modify] https://crrev.com/c7f0b95da92da5facfe659862224c22eaea8d9c4/components/leveldb_proto/proto_database_perftest.cc
[add] https://crrev.com/c7f0b95da92da5facfe659862224c22eaea8d9c4/components/leveldb_proto/proto_leveldb_wrapper.cc
[add] https://crrev.com/c7f0b95da92da5facfe659862224c22eaea8d9c4/components/leveldb_proto/proto_leveldb_wrapper.h
[modify] https://crrev.com/c7f0b95da92da5facfe659862224c22eaea8d9c4/components/leveldb_proto/testing/fake_db.h
[add] https://crrev.com/c7f0b95da92da5facfe659862224c22eaea8d9c4/components/leveldb_proto/unique_proto_database.h
[rename] https://crrev.com/c7f0b95da92da5facfe659862224c22eaea8d9c4/components/leveldb_proto/unique_proto_database_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/78e6d87067a730f7c827ee49f31eeb51ac06ba53

commit 78e6d87067a730f7c827ee49f31eeb51ac06ba53
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Mon Nov 05 17:09:47 2018

Add alternative ctor/Init for ProtoDatabase.

The Init function in ProtoDatabase currently assumes that we're dealing
with a unique proto database in that it takes a database directory and
leveldb_env::Options for opening a new DB.

In the case of a DB client using an underlying shared database, this
isn't necessary, so I've added a new Init function that only takes the
client name for UMA purposes and the Init callback, as well as adding
a constructor that takes the database dir and open options instead.
This allows clients in the new world can rely on just using the 2
parameter Init and not concern themselves with what version of the DB
they're using.

The old Init function is left for compatibility, and we ensure that the
new Init function isn't called after using the old constructor. A unit
test is added to test the logic that ensures we return false in the
InitCallback if this happens.

Bug: 870813
Change-Id: I7ec46228d964ddab3f4944163b91430a1cd72ac9
Reviewed-on: https://chromium-review.googlesource.com/c/1297465
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605366}
[modify] https://crrev.com/78e6d87067a730f7c827ee49f31eeb51ac06ba53/components/leveldb_proto/proto_database.h
[modify] https://crrev.com/78e6d87067a730f7c827ee49f31eeb51ac06ba53/components/leveldb_proto/testing/fake_db.h
[modify] https://crrev.com/78e6d87067a730f7c827ee49f31eeb51ac06ba53/components/leveldb_proto/unique_proto_database.h

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dd10ae173d8c2ea38d822418251dcd3add7de271

commit dd10ae173d8c2ea38d822418251dcd3add7de271
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Tue Nov 06 17:55:23 2018

Add metrics recording to ProtoLevelDBWrapper.

Adds metrics collection at the LevelDB wrapper level for ProtoDatabase,
so any current clients of ProtoDatabase will have new metrics recorded
for successes/failures and the error status of various LevelDB
operations.

To allow gathering metrics on the LevelDB status for failures,
the parameters to many of the functions have been modified so we can
get the leveldb::Status back in addition to the boolean return value.

Bug: 870813
Change-Id: I5d8175dbd4e3b3531532d2f3fc0b33ca155f1336
Reviewed-on: https://chromium-review.googlesource.com/c/1288904
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605725}
[modify] https://crrev.com/dd10ae173d8c2ea38d822418251dcd3add7de271/components/leveldb_proto/BUILD.gn
[modify] https://crrev.com/dd10ae173d8c2ea38d822418251dcd3add7de271/components/leveldb_proto/leveldb_database.cc
[modify] https://crrev.com/dd10ae173d8c2ea38d822418251dcd3add7de271/components/leveldb_proto/leveldb_database.h
[modify] https://crrev.com/dd10ae173d8c2ea38d822418251dcd3add7de271/components/leveldb_proto/proto_leveldb_wrapper.cc
[modify] https://crrev.com/dd10ae173d8c2ea38d822418251dcd3add7de271/components/leveldb_proto/proto_leveldb_wrapper.h
[add] https://crrev.com/dd10ae173d8c2ea38d822418251dcd3add7de271/components/leveldb_proto/proto_leveldb_wrapper_metrics.cc
[add] https://crrev.com/dd10ae173d8c2ea38d822418251dcd3add7de271/components/leveldb_proto/proto_leveldb_wrapper_metrics.h
[modify] https://crrev.com/dd10ae173d8c2ea38d822418251dcd3add7de271/components/leveldb_proto/unique_proto_database_unittest.cc
[modify] https://crrev.com/dd10ae173d8c2ea38d822418251dcd3add7de271/tools/metrics/histograms/histograms.xml

Project Member

Comment 12 by bugdroid1@chromium.org, Nov 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1

commit bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Wed Nov 28 21:32:51 2018

Add a ProtoDatabaseProvider as a keyed service.

Adds a ProtoDatabaseProvider(Factory) to provide ProtoDatabases with
the user directory, with ProtoDatabaseProvider as a
BrowserContextKeyedService.

Since ProtoDatabase is just an interface, and the underlying
implementation of it can vary from client to client, a
ProtoDatabaseWrapper that contains a pointer to a ProtoDatabase is
created, also built off ProtoDatabase.

Modifies the Feature Engagement Tracker to use the new provider for
its two databases.

Bug: 870813
Change-Id: I26376a14e8280605b7e3431862e5fe3ad6b9c8db
Reviewed-on: https://chromium-review.googlesource.com/c/1318053
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: edchin <edchin@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611878}
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/chrome/browser/BUILD.gn
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/chrome/browser/feature_engagement/tracker_factory.cc
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/feature_engagement/internal/persistent_availability_store.cc
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/feature_engagement/internal/persistent_availability_store_unittest.cc
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/feature_engagement/internal/persistent_event_store.cc
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/feature_engagement/internal/persistent_event_store.h
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/feature_engagement/internal/persistent_event_store_unittest.cc
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/feature_engagement/internal/tracker_impl.cc
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/feature_engagement/public/tracker.h
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/BUILD.gn
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/DEPS
[add] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/content/BUILD.gn
[add] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/content/DEPS
[add] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/content/proto_database_provider_factory.cc
[add] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/content/proto_database_provider_factory.h
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/leveldb_database.cc
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/leveldb_database.h
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/proto_database.h
[add] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/proto_database_provider.cc
[add] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/proto_database_provider.h
[add] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/proto_database_wrapper.h
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/proto_leveldb_wrapper.cc
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/proto_leveldb_wrapper.h
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/testing/fake_db.h
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/components/leveldb_proto/unique_proto_database.h
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/ios/chrome/browser/feature_engagement/BUILD.gn
[modify] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/ios/chrome/browser/feature_engagement/tracker_factory_util.mm
[add] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/ios/chrome/browser/leveldb_proto/BUILD.gn
[add] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/ios/chrome/browser/leveldb_proto/proto_database_provider_factory.h
[add] https://crrev.com/bc4ea7f7bc1f3bcad173bdf4edeb5d151ba25cc1/ios/chrome/browser/leveldb_proto/proto_database_provider_factory.mm

Project Member

Comment 13 by bugdroid1@chromium.org, Nov 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ea6f1cd42a311248e39b9fd94bb381bdc816854

commit 7ea6f1cd42a311248e39b9fd94bb381bdc816854
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Thu Nov 29 02:39:46 2018

Add SharedProtoDatabase and SharedProtoDatabaseClient.

Adds a SharedProtoDatabase that owns a LevelDB instance. The shared
database provides a way to get SharedProtoDatabaseClients to access
the shared DB with a unique namespace/prefix. The SharedProtoDatabase
instance is provided through the KeyedService ProtoDatabaseProvider
so the shared database is tied to the appropriate profile directory.

Bug: 870813
Change-Id: I433518c3fee4d37abcaa0c7efa94e7912da521ec
Reviewed-on: https://chromium-review.googlesource.com/c/1332009
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612000}
[modify] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/feature_engagement/internal/tracker_impl.cc
[modify] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/leveldb_proto/BUILD.gn
[modify] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/leveldb_proto/proto_database_provider.cc
[modify] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/leveldb_proto/proto_database_provider.h
[modify] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/leveldb_proto/proto_database_wrapper.h
[modify] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/leveldb_proto/proto_leveldb_wrapper.cc
[modify] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/leveldb_proto/proto_leveldb_wrapper.h
[add] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/leveldb_proto/shared_proto_database.cc
[add] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/leveldb_proto/shared_proto_database.h
[add] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/leveldb_proto/shared_proto_database_client.cc
[add] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/leveldb_proto/shared_proto_database_client.h
[add] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/leveldb_proto/shared_proto_database_client_unittest.cc
[add] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/leveldb_proto/shared_proto_database_unittest.cc
[modify] https://crrev.com/7ea6f1cd42a311248e39b9fd94bb381bdc816854/components/leveldb_proto/unique_proto_database.h

Project Member

Comment 14 by bugdroid1@chromium.org, Nov 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/768fdd5c7936570c0f39f1fed686a9b3709903ca

commit 768fdd5c7936570c0f39f1fed686a9b3709903ca
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Thu Nov 29 20:08:48 2018

Use composition in SharedProtoDatabaseClient instead of inheritance.

SharedProtoDatabaseClient inherited from UniqueProtoDatabase which was
potentially risky since we can accidentally use some of UniqueProtoDB's
functionality resulting in incorrect keys/entries from the database
because we're not stripping/adding prefixes.

We now inherit from ProtoDatabase to ensure that any functions added in
the future must also be implemented in SharedProtoDatabase.

Bug: 870813
Change-Id: I270afdb9dcffa0ba045756272a2f36af9cc93048
Reviewed-on: https://chromium-review.googlesource.com/c/1355423
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612331}
[modify] https://crrev.com/768fdd5c7936570c0f39f1fed686a9b3709903ca/components/leveldb_proto/shared_proto_database_client.h
[modify] https://crrev.com/768fdd5c7936570c0f39f1fed686a9b3709903ca/components/leveldb_proto/unique_proto_database.h

Project Member

Comment 15 by bugdroid1@chromium.org, Nov 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e8e8e4beb1c3a3e2e074381980e05e831a9c4e6e

commit e8e8e4beb1c3a3e2e074381980e05e831a9c4e6e
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Thu Nov 29 23:38:22 2018

Add IsCorrupt() to ProtoDatabase wrappers/clients.

Adds the ability to check for corruption via an IsCorrupt() function
that's pure virtual on the ProtoDatabase. IsCorrupt() is also introduced
in the ProtoLevelDBWrapper, and the corruption flag gets set on the Init
callback after checking status.

Unit tests changed slightly because the init callback needed to be put
into ProtoLevelDBWrapper instead of outside so it could modify
is_corrupt_, and the new use of the WeakPtr caused a race condition that
meant the test always failed to trigger the callback if we don't wait
for init to complete.

Bug: 870813
Change-Id: Ib00435dae8cdac490053fbfc6befb95094b1edec
Reviewed-on: https://chromium-review.googlesource.com/c/1355521
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612434}
[modify] https://crrev.com/e8e8e4beb1c3a3e2e074381980e05e831a9c4e6e/components/leveldb_proto/proto_database.h
[modify] https://crrev.com/e8e8e4beb1c3a3e2e074381980e05e831a9c4e6e/components/leveldb_proto/proto_leveldb_wrapper.cc
[modify] https://crrev.com/e8e8e4beb1c3a3e2e074381980e05e831a9c4e6e/components/leveldb_proto/proto_leveldb_wrapper.h
[modify] https://crrev.com/e8e8e4beb1c3a3e2e074381980e05e831a9c4e6e/components/leveldb_proto/shared_proto_database.cc
[modify] https://crrev.com/e8e8e4beb1c3a3e2e074381980e05e831a9c4e6e/components/leveldb_proto/shared_proto_database_client.h
[modify] https://crrev.com/e8e8e4beb1c3a3e2e074381980e05e831a9c4e6e/components/leveldb_proto/unique_proto_database.h
[modify] https://crrev.com/e8e8e4beb1c3a3e2e074381980e05e831a9c4e6e/components/leveldb_proto/unique_proto_database_unittest.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Dec 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d5c9a3aa4215bdd9090bea753bce961e60e70de

commit 5d5c9a3aa4215bdd9090bea753bce961e60e70de
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Wed Dec 12 22:11:12 2018

Fix SharedProtoDatabase WeakPtr dereferences/multiple init issues.

Introduces the concept of pending client initializations so that we
don't fail in odd ways if multiple initializations are in flight.

Also fixes some of the initialization PostTasks to ensure we're always
dereferencing our WeakPtrs on the same task runnerand ensuring that we
actually post our callbacks on the calling task runner.

Removes the WeakPtrFactory from ProtoLevelDBWrapper as well, which
involves a substantial refactoring to get DB init status back. A new
InitStatusCallback has been created so the 2 param Init and
InitWithDatabase calls give me an InitStatus. IsCorrupt was removed,
since setting the corruption state was the only thing that required
using a WeakPtr in the first place. This gives us the freedom to make
calls to the wrapper from any sequence regardless of what it was
created on, and not have the WeakPtrFactory cause problems when it's
destructed.

Bug: 912117,870813
Change-Id: Ic7931a543b4d3d09714184dfb335311130bc7667
Reviewed-on: https://chromium-review.googlesource.com/c/1364074
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616072}
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/feature_engagement/internal/persistent_availability_store.cc
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/feature_engagement/internal/persistent_availability_store_unittest.cc
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/feature_engagement/internal/persistent_event_store.cc
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/feature_engagement/internal/persistent_event_store.h
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/feature_engagement/internal/persistent_event_store_unittest.cc
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/proto_database.cc
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/proto_database.h
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/proto_database_provider.cc
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/proto_database_provider.h
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/proto_database_wrapper.h
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/proto_leveldb_wrapper.cc
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/proto_leveldb_wrapper.h
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/shared_proto_database.cc
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/shared_proto_database.h
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/shared_proto_database_client.cc
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/shared_proto_database_client.h
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/shared_proto_database_client_unittest.cc
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/shared_proto_database_unittest.cc
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/testing/fake_db.h
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/unique_proto_database.h
[modify] https://crrev.com/5d5c9a3aa4215bdd9090bea753bce961e60e70de/components/leveldb_proto/unique_proto_database_unittest.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Dec 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b7014be538ba9193e1531c799a6cf9f3d9cc00e

commit 3b7014be538ba9193e1531c799a6cf9f3d9cc00e
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Tue Dec 18 17:32:14 2018

Use SharedProtoDatabaseClient in ProtoDatabaseWrapper

Introduces the ability to get a SharedProtoDatabaseClient when using
ProtoDatabaseWrapper instead of strictly a UniqueProtoDatabase.

Includes bi-directional migration, and a new suite of tests for
ProtoDatabaseWrapper that ensure the flow works when getting either
type of database and that migration works in both directions.

Bug: 870813
Change-Id: I0cb47497dbdf98903a1bf17c68b0a4f6edcb7e14
Reviewed-on: https://chromium-review.googlesource.com/c/1354490
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617549}
[modify] https://crrev.com/3b7014be538ba9193e1531c799a6cf9f3d9cc00e/components/leveldb_proto/BUILD.gn
[add] https://crrev.com/3b7014be538ba9193e1531c799a6cf9f3d9cc00e/components/leveldb_proto/migration_delegate.h
[modify] https://crrev.com/3b7014be538ba9193e1531c799a6cf9f3d9cc00e/components/leveldb_proto/proto_database_provider.cc
[modify] https://crrev.com/3b7014be538ba9193e1531c799a6cf9f3d9cc00e/components/leveldb_proto/proto_database_provider.h
[modify] https://crrev.com/3b7014be538ba9193e1531c799a6cf9f3d9cc00e/components/leveldb_proto/proto_database_wrapper.h
[add] https://crrev.com/3b7014be538ba9193e1531c799a6cf9f3d9cc00e/components/leveldb_proto/proto_database_wrapper_unittest.cc
[modify] https://crrev.com/3b7014be538ba9193e1531c799a6cf9f3d9cc00e/components/leveldb_proto/shared_proto_database.cc
[modify] https://crrev.com/3b7014be538ba9193e1531c799a6cf9f3d9cc00e/components/leveldb_proto/shared_proto_database.h
[add] https://crrev.com/3b7014be538ba9193e1531c799a6cf9f3d9cc00e/components/leveldb_proto/shared_proto_database_client_list.cc
[add] https://crrev.com/3b7014be538ba9193e1531c799a6cf9f3d9cc00e/components/leveldb_proto/shared_proto_database_client_list.h
[add] https://crrev.com/3b7014be538ba9193e1531c799a6cf9f3d9cc00e/components/leveldb_proto/shared_proto_database_provider.cc
[add] https://crrev.com/3b7014be538ba9193e1531c799a6cf9f3d9cc00e/components/leveldb_proto/shared_proto_database_provider.h
[modify] https://crrev.com/3b7014be538ba9193e1531c799a6cf9f3d9cc00e/components/leveldb_proto/unique_proto_database_unittest.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Jan 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7baaeb37c979c55e8de067e3a82a8f06d33ec026

commit 7baaeb37c979c55e8de067e3a82a8f06d33ec026
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Thu Jan 03 18:58:02 2019

Introduce metadata database to SharedProtoDatabase.

The metadata database is primarily for storing corruption information
at the moment. Adds the creation/loading of entries from the metadata
DB on shared database init, and causes the database load to fail if
metadata can't be loaded, since that would be potentially dangerous.

Adds an async check for corruption information during
SharedProtoDatabaseClient init, and sets the corruption flag accordingly
so IsCorrupt() is correct.

Bug: 870813
Change-Id: Id24cd2c5c2aeed21d0e8bb108cc4126c3acb3049
Reviewed-on: https://chromium-review.googlesource.com/c/1356224
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619703}
[modify] https://crrev.com/7baaeb37c979c55e8de067e3a82a8f06d33ec026/components/leveldb_proto/BUILD.gn
[add] https://crrev.com/7baaeb37c979c55e8de067e3a82a8f06d33ec026/components/leveldb_proto/proto/shared_db_metadata.proto
[add] https://crrev.com/7baaeb37c979c55e8de067e3a82a8f06d33ec026/components/leveldb_proto/proto_database_wrapper.cc
[modify] https://crrev.com/7baaeb37c979c55e8de067e3a82a8f06d33ec026/components/leveldb_proto/proto_database_wrapper.h
[modify] https://crrev.com/7baaeb37c979c55e8de067e3a82a8f06d33ec026/components/leveldb_proto/proto_leveldb_wrapper.cc
[modify] https://crrev.com/7baaeb37c979c55e8de067e3a82a8f06d33ec026/components/leveldb_proto/proto_leveldb_wrapper.h
[modify] https://crrev.com/7baaeb37c979c55e8de067e3a82a8f06d33ec026/components/leveldb_proto/shared_proto_database.cc
[modify] https://crrev.com/7baaeb37c979c55e8de067e3a82a8f06d33ec026/components/leveldb_proto/shared_proto_database.h
[modify] https://crrev.com/7baaeb37c979c55e8de067e3a82a8f06d33ec026/components/leveldb_proto/shared_proto_database_client.cc
[modify] https://crrev.com/7baaeb37c979c55e8de067e3a82a8f06d33ec026/components/leveldb_proto/shared_proto_database_client.h
[modify] https://crrev.com/7baaeb37c979c55e8de067e3a82a8f06d33ec026/components/leveldb_proto/shared_proto_database_unittest.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Jan 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1528d239775ae0b3f889f43ccba2b5f514980d4b

commit 1528d239775ae0b3f889f43ccba2b5f514980d4b
Author: Siddhartha <ssid@chromium.org>
Date: Tue Jan 08 23:39:10 2019

leveldb_proto: Remove data from obsolete clients

If a database is no longer useful then the client name should be added
to obsolete clients list. This list exists to ensure that we do not
have prefix clashes between past, current and future databases. If a
feature forgets to cleanup the database, then previously we incurred
only storage overhead, but with shared database we will incur memory
overhead of loading all the data as well. So, this CL also adds
functionality of automatically cleaning up data from obsolete
databases.

BUG=870813

Change-Id: I5f6039b1e8c6e04ed0a450e30564b68847cc492e
Reviewed-on: https://chromium-review.googlesource.com/c/1388207
Commit-Queue: ssid <ssid@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620933}
[modify] https://crrev.com/1528d239775ae0b3f889f43ccba2b5f514980d4b/components/leveldb_proto/proto_database.h
[modify] https://crrev.com/1528d239775ae0b3f889f43ccba2b5f514980d4b/components/leveldb_proto/proto_database_provider.h
[modify] https://crrev.com/1528d239775ae0b3f889f43ccba2b5f514980d4b/components/leveldb_proto/proto_leveldb_wrapper.cc
[modify] https://crrev.com/1528d239775ae0b3f889f43ccba2b5f514980d4b/components/leveldb_proto/proto_leveldb_wrapper.h
[modify] https://crrev.com/1528d239775ae0b3f889f43ccba2b5f514980d4b/components/leveldb_proto/shared_proto_database.cc
[modify] https://crrev.com/1528d239775ae0b3f889f43ccba2b5f514980d4b/components/leveldb_proto/shared_proto_database.h
[modify] https://crrev.com/1528d239775ae0b3f889f43ccba2b5f514980d4b/components/leveldb_proto/shared_proto_database_client.cc
[modify] https://crrev.com/1528d239775ae0b3f889f43ccba2b5f514980d4b/components/leveldb_proto/shared_proto_database_client.h
[modify] https://crrev.com/1528d239775ae0b3f889f43ccba2b5f514980d4b/components/leveldb_proto/shared_proto_database_client_list.cc
[modify] https://crrev.com/1528d239775ae0b3f889f43ccba2b5f514980d4b/components/leveldb_proto/shared_proto_database_client_list.h
[modify] https://crrev.com/1528d239775ae0b3f889f43ccba2b5f514980d4b/components/leveldb_proto/shared_proto_database_client_unittest.cc
[modify] https://crrev.com/1528d239775ae0b3f889f43ccba2b5f514980d4b/components/leveldb_proto/unique_proto_database.h
[modify] https://crrev.com/1528d239775ae0b3f889f43ccba2b5f514980d4b/components/leveldb_proto/unique_proto_database_unittest.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Jan 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d5523223c2bcf3f6eaf6418fcf769f1c59a7c435

commit d5523223c2bcf3f6eaf6418fcf769f1c59a7c435
Author: Salvador Guerrero <salg@google.com>
Date: Wed Jan 09 18:39:47 2019

Add functions to ProtoDatabase to get entries within a range of keys

Some clients would get data within a range of keys (For paging or
retrieving data in a date range) by calling LoadEntriesWithFilter,
which ends up iterating through the entire database.

A new function is added to ProtoDatabase that loads all keys and
entries in a specified key range (range includes both start and end key)

Bug: 870813

Change-Id: I4a52a24712271fe597c674f5a5a7f4d2cfd5277e
Reviewed-on: https://chromium-review.googlesource.com/c/1391358
Commit-Queue: Salvador Guerrero <salg@google.com>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621238}
[modify] https://crrev.com/d5523223c2bcf3f6eaf6418fcf769f1c59a7c435/components/leveldb_proto/leveldb_database.cc
[modify] https://crrev.com/d5523223c2bcf3f6eaf6418fcf769f1c59a7c435/components/leveldb_proto/leveldb_database.h
[modify] https://crrev.com/d5523223c2bcf3f6eaf6418fcf769f1c59a7c435/components/leveldb_proto/proto_database.h
[modify] https://crrev.com/d5523223c2bcf3f6eaf6418fcf769f1c59a7c435/components/leveldb_proto/proto_database_wrapper.h
[modify] https://crrev.com/d5523223c2bcf3f6eaf6418fcf769f1c59a7c435/components/leveldb_proto/proto_leveldb_wrapper.h
[modify] https://crrev.com/d5523223c2bcf3f6eaf6418fcf769f1c59a7c435/components/leveldb_proto/shared_proto_database_client.h
[modify] https://crrev.com/d5523223c2bcf3f6eaf6418fcf769f1c59a7c435/components/leveldb_proto/shared_proto_database_client_unittest.cc
[modify] https://crrev.com/d5523223c2bcf3f6eaf6418fcf769f1c59a7c435/components/leveldb_proto/unique_proto_database.h
[modify] https://crrev.com/d5523223c2bcf3f6eaf6418fcf769f1c59a7c435/components/leveldb_proto/unique_proto_database_unittest.cc

Project Member

Comment 21 by bugdroid1@chromium.org, Jan 17 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e5a5b1a156da6401d52780be453806b4b1f26111

commit e5a5b1a156da6401d52780be453806b4b1f26111
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Thu Jan 17 06:54:37 2019

leveldb_proto: Reorganize files to clarify public API

The ProtoDatabase, ProtoDatabaseProvider and the client list for the
shared ProtoDatabase has been moved to
//components/leveldb_proto/public, and the rest has been moved to
//components/leveldb_proto/internal.

Since the public API is not cleansed for internal references in this CL,
the GN-targets are still the same as before, but this should be fixed
as a follow-up, including setting visibility for the GN targets to
ensure only the public API is used by clients.

Previously, it was possible to directly create a ProtoDatabaseImpl,
which has been renamed to UniqueProtoDatabase. This CL removes the
ProtoDatabaseImpl, and moves all usages to instead just create a
generic ProtoDatabase from the ProtoDatabaseProvider. This is to make
move to getting either a shared or a unique database easier in the
future, but for now, all clients directly invoke the static create
method which still provides their own UniqueProtoDatabase.

TBR=sdefresne@chromium.org,peter@chromium.org,chcunningham@chromium.org

Bug: 870813
Change-Id: Iedeb728eb0b9523d765b360c125a671522567abc
Reviewed-on: https://chromium-review.googlesource.com/c/1406295
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Cait Phillips <caitkp@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623611}
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/chrome/browser/android/usage_stats/usage_stats_database.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/chrome/browser/android/usage_stats/usage_stats_database.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/chrome/browser/dom_distiller/dom_distiller_service_factory.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/chrome/browser/push_messaging/budget_database.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/chrome/browser/push_messaging/budget_database.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/chrome/browser/push_messaging/budget_database_unittest.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/BUILD.gn
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/autofill/core/browser/legacy_strike_database.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/autofill/core/browser/legacy_strike_database.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/autofill/core/browser/strike_database.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/autofill/core/browser/strike_database.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/autofill/core/browser/strike_database_integrator_base.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/dom_distiller/core/dom_distiller_store.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/dom_distiller/standalone/content_extractor_browsertest.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/download/content/factory/download_service_factory.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/download/database/download_db_impl.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/download/database/download_db_impl.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/download/internal/background_service/download_store.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/download/internal/background_service/download_store.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feature_engagement/internal/persistent_availability_store.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feature_engagement/internal/persistent_availability_store.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feature_engagement/internal/persistent_availability_store_unittest.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feature_engagement/internal/persistent_event_store.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feature_engagement/internal/persistent_event_store.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feature_engagement/internal/persistent_event_store_unittest.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feature_engagement/internal/tracker_impl.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feed/core/feed_content_database.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feed/core/feed_content_database.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feed/core/feed_image_database.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feed/core/feed_image_database.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feed/core/feed_image_manager_unittest.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feed/core/feed_journal_database.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/feed/core/feed_journal_database.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/gcm_driver/crypto/gcm_key_store.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/gcm_driver/crypto/gcm_key_store_unittest.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/gcm_driver/instance_id/instance_id_driver_unittest.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/image_fetcher/core/cache/image_metadata_store_leveldb.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/image_fetcher/core/cache/image_metadata_store_leveldb.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/image_fetcher/core/cached_image_fetcher_unittest.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/BUILD.gn
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/content/proto_database_provider_factory.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/leveldb_database.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/leveldb_database.h
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/migration_delegate.h
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/proto/shared_db_metadata.proto
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/proto_database_perftest.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/proto_database_wrapper.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/proto_database_wrapper.h
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/proto_database_wrapper_unittest.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/proto_leveldb_wrapper.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/proto_leveldb_wrapper.h
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/proto_leveldb_wrapper_metrics.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/proto_leveldb_wrapper_metrics.h
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/shared_proto_database.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/shared_proto_database.h
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/shared_proto_database_client.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/shared_proto_database_client.h
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/shared_proto_database_client_unittest.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/shared_proto_database_provider.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/shared_proto_database_provider.h
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/shared_proto_database_unittest.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/unique_proto_database.h
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/internal/unique_proto_database_unittest.cc
[delete] https://crrev.com/edbdbdfc3c8eda1589ce9c160a0b47038738e1f2/components/leveldb_proto/proto_database_impl.h
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/public/proto_database.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/public/proto_database.h
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/public/proto_database_provider.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/public/proto_database_provider.h
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/public/shared_proto_database_client_list.cc
[rename] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/public/shared_proto_database_client_list.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/testing/fake_db.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/leveldb_proto/testing/proto/test_db.proto
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/ntp_snippets/remote/cached_image_fetcher_unittest.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/ntp_snippets/remote/remote_suggestions_database.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/ntp_snippets/remote/remote_suggestions_database.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/previews/content/hint_cache_leveldb_store.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/components/previews/content/hint_cache_leveldb_store.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/ios/chrome/browser/dom_distiller/dom_distiller_service_factory.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/ios/chrome/browser/leveldb_proto/proto_database_provider_factory.mm
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/media/capabilities/in_memory_video_decode_stats_db_impl.h
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/media/capabilities/video_decode_stats_db_impl.cc
[modify] https://crrev.com/e5a5b1a156da6401d52780be453806b4b1f26111/media/capabilities/video_decode_stats_db_impl.h

Project Member

Comment 22 by bugdroid1@chromium.org, Jan 18 (5 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f0ab9525ea179a3814f8ff78b4e7702ae27791bb

commit f0ab9525ea179a3814f8ff78b4e7702ae27791bb
Author: Siddhartha <ssid@chromium.org>
Date: Fri Jan 18 01:27:34 2019

Remove weak ptr from SharedProtoDatabase

SharedProtoDatabase posted task to background task runner to delete it's
weap_ptr. But, if the object is deleted without invalidating weak ptr
then the posted tasks can still access a deleted object. To fix this,
use scoped_refptr to post all the tasks.

BUG=870813

Change-Id: Ieea4d04792d9c95558ca613da426cc616ce12c61
Reviewed-on: https://chromium-review.googlesource.com/c/1416233
Commit-Queue: ssid <ssid@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623941}
[modify] https://crrev.com/f0ab9525ea179a3814f8ff78b4e7702ae27791bb/components/leveldb_proto/internal/shared_proto_database.cc
[modify] https://crrev.com/f0ab9525ea179a3814f8ff78b4e7702ae27791bb/components/leveldb_proto/internal/shared_proto_database.h
[modify] https://crrev.com/f0ab9525ea179a3814f8ff78b4e7702ae27791bb/components/leveldb_proto/internal/shared_proto_database_client_unittest.cc
[modify] https://crrev.com/f0ab9525ea179a3814f8ff78b4e7702ae27791bb/components/leveldb_proto/internal/shared_proto_database_unittest.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Jan 18 (4 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f3b979dde378e52688c72c55de4f7dd110dafba

commit 7f3b979dde378e52688c72c55de4f7dd110dafba
Author: Salvador Guerrero <salg@google.com>
Date: Fri Jan 18 23:22:31 2019

Removed reference to leveldb_database.h from proto_database.h

proto_database.h is going to be the public interface for leveldb_proto,
but it had a reference to internal file leveldb_database.h, this CL
removes that reference and moves some code that should be public to its
right place.

Moved function InitWithDatabase from leveldb_database.h to
UniqueProtoDatabase, as it is used only internally by that class

TBR=sclittle@chromium.org

Bug: 870813
Change-Id: I215e290ef3b803b9ffb792a23bf1ec19bb173ced
Reviewed-on: https://chromium-review.googlesource.com/c/1407667
Reviewed-by: Scott Little <sclittle@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Commit-Queue: Salvador Guerrero <salg@google.com>
Cr-Commit-Position: refs/heads/master@{#624348}
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/leveldb_proto/internal/leveldb_database.h
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/leveldb_proto/internal/proto_database_perftest.cc
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/leveldb_proto/internal/proto_database_wrapper.h
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/leveldb_proto/internal/proto_leveldb_wrapper.cc
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/leveldb_proto/internal/proto_leveldb_wrapper.h
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/leveldb_proto/internal/shared_proto_database_client.cc
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/leveldb_proto/internal/shared_proto_database_client.h
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/leveldb_proto/internal/shared_proto_database_client_unittest.cc
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/leveldb_proto/internal/unique_proto_database.h
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/leveldb_proto/internal/unique_proto_database_unittest.cc
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/leveldb_proto/public/proto_database.cc
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/leveldb_proto/public/proto_database.h
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/leveldb_proto/testing/fake_db.h
[modify] https://crrev.com/7f3b979dde378e52688c72c55de4f7dd110dafba/components/previews/content/hint_cache_leveldb_store.cc

Sign in to add a comment