Second try at splitting WebDataService (minus ownership changes)

Second try at splitting WebDataService (minus ownership changes)

Split Database handling into a separate class, but keep it owned by WebDataService, to avoid it disappearing out from under us (crbug.com/181628).

TBR=ben@chromium.org,sky@chromium.org
TEST= unit_tests
BUG=181277
COLLABORATOR=joi@chromium.org


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188935
Rolled back as r188951. Relanding with fixes.

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

[Cait (Slow), 2013-03-15 00:57:23]

David: I ended up having to revert r186824 last night (the first attempt at
splitting DB handling from WDS). This patch is basically the same as before,
except that the WebDatabaseService is owned by the WebDataService, which I think
is how it has to be as long as WebDataService is refcounted. PTAL.

Joi: FYI, and to confirm that this approach will work with your in flight
changes.

Thanks,
Cait

[Jói, 2013-03-15 17:07:58]

I took a quick look and I'm pretty sure this works fine with the other changes
that are in flight.  Am I right that this is very close to the change you
reverted, with the main difference being the difference in ownership?

Cheers,
Jói

[dhollowa, 2013-03-15 18:56:20]

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_data_service.cc (right):

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:87: wdbs_.reset(NULL);
Not needed.  scoped_ptr starts out NULL.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:115:
Bind(&WebDataService::OnDatabaseInit, base::Unretained(this)));
Why Unretained here?

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:115:
Bind(&WebDataService::OnDatabaseInit, base::Unretained(this)));
The name |OnDatabaseInit| had me a little confused as I usually think of On*
methods to be called via a PostTask or IPC.

I wonder if DatabaseInitOnDB might be a better name?  Hmm.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:145: return wdbs_->GetDatabase();
What is the contract with the caller here?  Should there be a
DCHECK(wdbs_.get())?

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:331:
DCHECK(autocomplete_syncable_service_ == NULL);
DCHECK(!autocomplete_syncable_service_) or DCHECK_EQ

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:332:
DCHECK(autofill_profile_syncable_service_ == NULL);
DCHECK(!autofill_profile_syncable_service_) or DCHECK_EQ

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:357: if (status == sql::INIT_OK) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
Would be helpful here, more as doc than anything.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:730:
WebDataService::RemoveAutofillProfilesAndCreditCardsModifiedBetweenImpl(
nit: This line should not be indented.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_data_service.h (right):

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.h:143: virtual bool IsDatabaseLoaded();
Doc please.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.h:144: virtual WebDatabase*
GetDatabase();
Doc please.  When can/should this be called?  Is caller responsible for checking
IsDatabaseLoaded prior to calling this?

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_database_service.cc (right):

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:32: 
nit: Remove empty line.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:35: // Initializes the database
and notifies caller via callback when complete.
Please mention that callback is called synchronously on DB thread.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:39: // Opens the database file
from the profile path, if an init has not yet been
Remove comma in "path, if".

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:58: return request_manager_; }
} on new line

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:60: WebDatabase* database()
{return db_.get();}
nit: space after { and before }

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:107: db_.reset(NULL);
Not needed.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:189: wdbs_internal_ = NULL;
Not needed.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:193: if (!wdbs_internal_) {
nit: No need for {}

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:204: if (wdbs_internal_) {
Please change to the form:
  if (!wdbs_internal_)
    return;
  ... the rest ...

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:212: if (wdbs_internal_) {
Please change to the form:
  if (!wdbs_internal_)
    return;
  ... the rest ...

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:219: WebDatabase*
WebDatabaseService::GetDatabase() const {
Consider changing these exceptional methods to *OnDB, eg. GetDatabaseOnDB.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:221: if (wdbs_internal_)
Please change to the form:
  if (!wdbs_internal_)
    return;
  ... the rest ...

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:266: if (wdbs_internal_)
Please change to the form:
  if (!wdbs_internal_)
    return;
  ... the rest ...

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_database_service.h (right):

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.h:50: // Retrieve a
WebDatabaseService for the given context.
What is this comment referencing?

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.h:92:
scoped_refptr<WebDatabaseServiceInternal> wdbs_internal_;
Please document ownership an lifetime of this instance.  I.e. the primary owner
is |WebDatabaseService| but is refcounted because PostTask on DB thread may
outlive us.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.h:93: 
nit: Remove empty line.

[Cait (Slow), 2013-03-15 20:43:59]

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_data_service.cc (right):

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:87: wdbs_.reset(NULL);
On 2013/03/15 18:56:21, dhollowa wrote:
> Not needed.  scoped_ptr starts out NULL.

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:115:
Bind(&WebDataService::OnDatabaseInit, base::Unretained(this)));
On 2013/03/15 18:56:21, dhollowa wrote:
> Why Unretained here?

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:115:
Bind(&WebDataService::OnDatabaseInit, base::Unretained(this)));
On 2013/03/15 18:56:21, dhollowa wrote:
> The name |OnDatabaseInit| had me a little confused as I usually think of On*
> methods to be called via a PostTask or IPC.
> 
> I wonder if DatabaseInitOnDB might be a better name?  Hmm.

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:145: return wdbs_->GetDatabase();
On 2013/03/15 18:56:21, dhollowa wrote:
> What is the contract with the caller here?  Should there be a
> DCHECK(wdbs_.get())?

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:331:
DCHECK(autocomplete_syncable_service_ == NULL);
On 2013/03/15 18:56:21, dhollowa wrote:
> DCHECK(!autocomplete_syncable_service_) or DCHECK_EQ

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:332:
DCHECK(autofill_profile_syncable_service_ == NULL);
On 2013/03/15 18:56:21, dhollowa wrote:
> DCHECK(!autofill_profile_syncable_service_) or DCHECK_EQ

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:357: if (status == sql::INIT_OK) {
On 2013/03/15 18:56:21, dhollowa wrote:
> DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
> Would be helpful here, more as doc than anything.

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:730:
WebDataService::RemoveAutofillProfilesAndCreditCardsModifiedBetweenImpl(
On 2013/03/15 18:56:21, dhollowa wrote:
> nit: This line should not be indented.

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_data_service.h (right):

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.h:143: virtual bool IsDatabaseLoaded();
On 2013/03/15 18:56:21, dhollowa wrote:
> Doc please.

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.h:144: virtual WebDatabase*
GetDatabase();
On 2013/03/15 18:56:21, dhollowa wrote:
> Doc please.  When can/should this be called?  Is caller responsible for
checking
> IsDatabaseLoaded prior to calling this?

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_database_service.cc (right):

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:32: 
On 2013/03/15 18:56:21, dhollowa wrote:
> nit: Remove empty line.

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:35: // Initializes the database
and notifies caller via callback when complete.
On 2013/03/15 18:56:21, dhollowa wrote:
> Please mention that callback is called synchronously on DB thread.

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:39: // Opens the database file
from the profile path, if an init has not yet been
On 2013/03/15 18:56:21, dhollowa wrote:
> Remove comma in "path, if".

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:58: return request_manager_; }
On 2013/03/15 18:56:21, dhollowa wrote:
> } on new line

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:60: WebDatabase* database()
{return db_.get();}
On 2013/03/15 18:56:21, dhollowa wrote:
> nit: space after { and before }

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:107: db_.reset(NULL);
On 2013/03/15 18:56:21, dhollowa wrote:
> Not needed.

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:189: wdbs_internal_ = NULL;
On 2013/03/15 18:56:21, dhollowa wrote:
> Not needed.

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:193: if (!wdbs_internal_) {
On 2013/03/15 18:56:21, dhollowa wrote:
> nit: No need for {}

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:204: if (wdbs_internal_) {
On 2013/03/15 18:56:21, dhollowa wrote:
> Please change to the form:
>   if (!wdbs_internal_)
>     return;
>   ... the rest ...

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:212: if (wdbs_internal_) {
On 2013/03/15 18:56:21, dhollowa wrote:
> Please change to the form:
>   if (!wdbs_internal_)
>     return;
>   ... the rest ...

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:219: WebDatabase*
WebDatabaseService::GetDatabase() const {
On 2013/03/15 18:56:21, dhollowa wrote:
> Consider changing these exceptional methods to *OnDB, eg. GetDatabaseOnDB.

Done. Changed WebDatabaseService method names. Will change WebDataService in a
later CL as it touches serveral files.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:221: if (wdbs_internal_)
On 2013/03/15 18:56:21, dhollowa wrote:
> Please change to the form:
>   if (!wdbs_internal_)
>     return;
>   ... the rest ...

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:266: if (wdbs_internal_)
On 2013/03/15 18:56:21, dhollowa wrote:
> Please change to the form:
>   if (!wdbs_internal_)
>     return;
>   ... the rest ...

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_database_service.h (right):

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.h:50: // Retrieve a
WebDatabaseService for the given context.
On 2013/03/15 18:56:21, dhollowa wrote:
> What is this comment referencing?

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.h:92:
scoped_refptr<WebDatabaseServiceInternal> wdbs_internal_;
On 2013/03/15 18:56:21, dhollowa wrote:
> Please document ownership an lifetime of this instance.  I.e. the primary
owner
> is |WebDatabaseService| but is refcounted because PostTask on DB thread may
> outlive us.

Done.

https://codereview.chromium.org/12871006/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.h:93: 
On 2013/03/15 18:56:21, dhollowa wrote:
> nit: Remove empty line.

Done.

[dhollowa, 2013-03-15 23:50:30]

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_data_service.cc (right):

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:113: wdbs_->LoadDatabase(
nit: Fits on one line.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:121: if (wdbs_.get())
if (wdbs_)

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:126: if (wdbs_.get())
if (wdbs_)

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:131: if (wdbs_.get())
if (wdbs_)

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:144: if (!wdbs_.get())
if (!wdbs_)

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:379: 
nit: Extra line.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_database_service.cc (right):

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:135: if (db_.get() &&
init_status_ == sql::INIT_OK)
if(db_ &&...

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:148: if (db_.get() &&
init_status_ == sql::INIT_OK && !request->IsCancelled()) {
if(db_ &&...

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:160: if (db_.get() &&
init_status_ == sql::INIT_OK && !request->IsCancelled()) {
if(db_ &&...

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:171: if (db_.get() &&
init_status_ == sql::INIT_OK) {
if(db_ &&...

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:172: db_->CommitTransaction();
nit: Indent

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:226: const
tracked_objects::Location& from_here,
nit: Indent

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:242: const
tracked_objects::Location& from_here,
nit: Indent

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_database_service.h (right):

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.h:91:
scoped_refptr<WebDatabaseServiceInternal> wdbs_internal_;
Sorry to be going off on naming here.  But "WebDataServiceBackend" would be more
communicative and consistent with existing Chrome convention.  Eg.
HistoryBackend, ShortcutsBackend, SessionBackend, etc..  How about it?

[Cait (Slow), 2013-03-16 21:34:21]

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_data_service.cc (right):

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:113: wdbs_->LoadDatabase(
On 2013/03/15 23:50:31, dhollowa wrote:
> nit: Fits on one line.

Done.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:121: if (wdbs_.get())
On 2013/03/15 23:50:31, dhollowa wrote:
> if (wdbs_)

Done.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:126: if (wdbs_.get())
On 2013/03/15 23:50:31, dhollowa wrote:
> if (wdbs_)

Done.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:131: if (wdbs_.get())
On 2013/03/15 23:50:31, dhollowa wrote:
> if (wdbs_)

Done.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:144: if (!wdbs_.get())
On 2013/03/15 23:50:31, dhollowa wrote:
> if (!wdbs_)

Done.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_data_service.cc:379: 
On 2013/03/15 23:50:31, dhollowa wrote:
> nit: Extra line.

Done.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_database_service.cc (right):

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:135: if (db_.get() &&
init_status_ == sql::INIT_OK)
On 2013/03/15 23:50:31, dhollowa wrote:
> if(db_ &&...

Done.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:148: if (db_.get() &&
init_status_ == sql::INIT_OK && !request->IsCancelled()) {
On 2013/03/15 23:50:31, dhollowa wrote:
> if(db_ &&...

Done.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:160: if (db_.get() &&
init_status_ == sql::INIT_OK && !request->IsCancelled()) {
On 2013/03/15 23:50:31, dhollowa wrote:
> if(db_ &&...

Done.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:171: if (db_.get() &&
init_status_ == sql::INIT_OK) {
On 2013/03/15 23:50:31, dhollowa wrote:
> if(db_ &&...

Done.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:172: db_->CommitTransaction();
On 2013/03/15 23:50:31, dhollowa wrote:
> nit: Indent

Done.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:226: const
tracked_objects::Location& from_here,
On 2013/03/15 23:50:31, dhollowa wrote:
> nit: Indent

Done.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.cc:242: const
tracked_objects::Location& from_here,
On 2013/03/15 23:50:31, dhollowa wrote:
> nit: Indent

Done.

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_database_service.h (right):

https://codereview.chromium.org/12871006/diff/35001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_service.h:91:
scoped_refptr<WebDatabaseServiceInternal> wdbs_internal_;
On 2013/03/15 23:50:31, dhollowa wrote:
> Sorry to be going off on naming here.  But "WebDataServiceBackend" would be
more
> communicative and consistent with existing Chrome convention.  Eg.
> HistoryBackend, ShortcutsBackend, SessionBackend, etc..  How about it?

Done.

[Jói, 2013-03-17 08:58:37]

Thanks for keeping this going over the weekend Cait. I took a better look and
this LGTM.

Cheers,
Jói

[dhollowa, 2013-03-18 17:29:28]

LGTM.  Thanks.

[commit-bot: I haz the power, 2013-03-18 18:43:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/caitkp@chromium.org/12871006/56001

[commit-bot: I haz the power, 2013-03-18 20:08:13]

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

[commit-bot: I haz the power, 2013-03-18 20:57:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/caitkp@chromium.org/12871006/54003

[commit-bot: I haz the power, 2013-03-19 03:18:49]

Change committed as 188935

[Jói, 2013-03-19 16:12:12]

Committed patchset #11 manually as r189030 (presubmit successful).