Eliminate the archived history database and clean up related code.

Eliminate the archived history database and clean up related code.

More specifically, this change list:
 * Removes all code related to the archived history database, and removes all mentions of it from comments.
 * Adds some code to purge the legacy database file on disk on first start-up, plus a unit test to check that this works.
 * Removes a migration path from M17 which was responsible for migrating some components, all of which have been unimplemented since then, except for the archived database, which, however, is now being removed.
 * Removes a tiny amount of code and some mentions of a "supplementary URL index" on the URLs table (this was unimplemented a long time ago). This is needed because otherwise some updated comments would be confusing.

BUG=359377
TBR=pkasting@chromium.org

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

[engedy, 2014-04-16 18:08:09]

@Scott: please take a look.

@Brett, Peter: FYI.

https://codereview.chromium.org/235863023/diff/30001/chrome/browser/history/h...
File chrome/browser/history/history_backend.cc (left):

https://codereview.chromium.org/235863023/diff/30001/chrome/browser/history/h...
chrome/browser/history/history_backend.cc:1313: if (url_db == db_.get()) {
Note: I think this should have been a non-equality here. However, from now on,
we will always work with the main DB, so line 1297 should always fetch the
up-to-date record, therefore this part shall no longer be needed.

https://codereview.chromium.org/235863023/diff/30001/chrome/browser/history/h...
File chrome/browser/history/history_database.cc (left):

https://codereview.chromium.org/235863023/diff/30001/chrome/browser/history/h...
chrome/browser/history/history_database.cc:223: // We also add the supplementary
URL indices at this point. This index is
Note: There used to be a CreateSupplimentaryURLIndices() function which created
an index on the FavIconID column in the URLs table. There had been no such thing
for quite a while now, so removing this too.

https://codereview.chromium.org/235863023/diff/30001/chrome/browser/history/h...
chrome/browser/history/history_database.cc:459: // Erase all the full text index
files. These will take a while to update and
Note: There had been no no full text index anymore, so removing this.

https://codereview.chromium.org/235863023/diff/30001/chrome/browser/history/h...
File chrome/browser/history/history_database.h (left):

https://codereview.chromium.org/235863023/diff/30001/chrome/browser/history/h...
chrome/browser/history/history_database.h:141: // Returns true if the history
backend should erase the full text search
Note: Given that we no longer had full text search, the Archived DB was the sole
user of this flag, so I removed this too.

https://codereview.chromium.org/235863023/diff/30001/chrome/browser/history/u...
File chrome/browser/history/url_database.cc (left):

https://codereview.chromium.org/235863023/diff/30001/chrome/browser/history/u...
chrome/browser/history/url_database.cc:237: // are created by
HistoryDatabase::RecreateAllButStarAndURLTables().
Note: Removing this as there is no CreateSupplimentaryURLIndices() anymore.

Interestingly, RecreateAllTablesButURL() had not always been called after this
function, so there had been cases when the FavIcon index would not have been
created.

However, this is no longer an issue, as we only have a single index on the URLs
table, which is created by CreateMainURLIndex().

https://codereview.chromium.org/235863023/diff/30001/chrome/browser/history/u...
File chrome/browser/history/url_database.h (left):

https://codereview.chromium.org/235863023/diff/30001/chrome/browser/history/u...
chrome/browser/history/url_database.h:115: // This function does not create the
supplimentary indices. It is virtual so
Note: Removing this as there is no CreateSupplimentaryURLIndices() anymore.
Also, this function is not actually overriden by the main DB.

[sky, 2014-04-16 19:18:07]

I'm redirecting this to Brett
+brettw
-sky

[Peter Kasting, 2014-04-16 19:48:50]

Hold off on this for a couple days as I'm about to land some nontrivial changes
to the ExpireHistoryBackend and you'll want to sync up with them.  (I'm pretty
sure they will still be necessary in the post-ArchivedDatabase world.)

[engedy, 2014-04-16 20:19:21]

On 2014/04/16 19:48:50, Peter Kasting wrote:
> Hold off on this for a couple days as I'm about to land some nontrivial
changes
> to the ExpireHistoryBackend and you'll want to sync up with them.  (I'm pretty
> sure they will still be necessary in the post-ArchivedDatabase world.)

Sure thing.

[engedy, 2014-04-23 17:52:15]

On 2014/04/16 20:19:21, engedy wrote:
> On 2014/04/16 19:48:50, Peter Kasting wrote:
> > Hold off on this for a couple days as I'm about to land some nontrivial
> changes
> > to the ExpireHistoryBackend and you'll want to sync up with them.  (I'm
pretty
> > sure they will still be necessary in the post-ArchivedDatabase world.)
> 
> Sure thing.

Peter, do you have any more CLs in the pipeline, or shall we proceed with
rebasing/reviewing this CL?

[Peter Kasting, 2014-04-23 17:55:23]

On 2014/04/23 17:52:15, engedy wrote:
> Peter, do you have any more CLs in the pipeline, or shall we proceed with
> rebasing/reviewing this CL?

Nope, go for it.

[engedy, 2014-05-28 15:22:30]

@Peter, @Brett: I have finally had time to rework this. Please take a look.

[engedy, 2014-06-03 09:57:16]

On 2014/05/28 15:22:30, engedy wrote:
> @Peter, @Brett: I have finally had time to rework this. Please take a look.

Friendly ping.

[brettw, 2014-06-03 17:08:26]

lgtm

https://codereview.chromium.org/235863023/diff/70001/chrome/browser/history/h...
File chrome/browser/history/history_backend.cc (right):

https://codereview.chromium.org/235863023/diff/70001/chrome/browser/history/h...
chrome/browser/history/history_backend.cc:615: // used for much, and
consequently has been removed as of M37.
Can you add a bug and TODO here to remove this code after, say, the end of 2014?
(so people have a chance to get updated to a version that deletes the file
before).

[Peter Kasting, 2014-06-03 18:03:48]

No need for my review, Brett's review is sufficient.

[engedy, 2014-06-05 10:52:55]

I have also found a few more instances where the we talked about things getting
"archived", I have rephrased each of these to talk about things getting
"expired".

https://codereview.chromium.org/235863023/diff/70001/chrome/browser/history/h...
File chrome/browser/history/history_backend.cc (right):

https://codereview.chromium.org/235863023/diff/70001/chrome/browser/history/h...
chrome/browser/history/history_backend.cc:615: // used for much, and
consequently has been removed as of M37.
On 2014/06/03 17:08:26, brettw wrote:
> Can you add a bug and TODO here to remove this code after, say, the end of
2014?
> (so people have a chance to get updated to a version that deletes the file
> before).

Done.

[engedy, 2014-06-05 11:03:04]

@Drew, could you PTAL at:
 * c/b/sync for OWNER's appproval,
 * and also double check that removing the archived database will not lead to
any explosions in the typed URL syncing code? (Especially the changes in
HistoryBackend::AddPagesWithDetails.)

[Andrew T Wilson (Slow), 2014-06-05 11:44:43]

lgtm. I can't guarantee that changes of this magnitude to the history code won't
cause typed url sync to blow up (it's complex!), but we have decent code
coverage so I think we're fine.

https://codereview.chromium.org/235863023/diff/110001/chrome/browser/history/...
File chrome/browser/history/history_backend.cc (left):

https://codereview.chromium.org/235863023/diff/110001/chrome/browser/history/...
chrome/browser/history/history_backend.cc:871:
&modified_in_archive->changed_urls);
Hmm, we're no longer notifying the sync subsystem of archived URLs. I actually
think this is OK since we ignore those anyway.

[engedy, 2014-06-05 12:17:58]

As discussed off-line, we will keep a close eye on metrics, and keep in touch
with the sync folks to make sure we detect it early if anything starts burning.

https://codereview.chromium.org/235863023/diff/110001/chrome/browser/history/...
File chrome/browser/history/history_backend.cc (left):

https://codereview.chromium.org/235863023/diff/110001/chrome/browser/history/...
chrome/browser/history/history_backend.cc:871:
&modified_in_archive->changed_urls);
On 2014/06/05 11:44:44, Andrew T Wilson wrote:
> Hmm, we're no longer notifying the sync subsystem of archived URLs. I actually
> think this is OK since we ignore those anyway.

Thanks for looking into this!

[engedy, 2014-06-05 12:18:46]

The CQ bit was checked by engedy@chromium.org

[engedy, 2014-06-05 12:19:31]

The CQ bit was unchecked by engedy@chromium.org

[engedy, 2014-06-05 12:23:17]

On 2014/06/05 12:19:31, engedy wrote:
> The CQ bit was unchecked by mailto:engedy@chromium.org

@Peter, it seems that I will need your sign-off on this after all as OWNER. I
discovered a comment in c/b/autocomplete that needed updating. I have added you
as TBR.

[engedy, 2014-06-05 12:23:29]

The CQ bit was checked by engedy@chromium.org

[commit-bot: I haz the power, 2014-06-05 12:24:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/engedy@chromium.org/235863023/110001

[commit-bot: I haz the power, 2014-06-05 16:44:41]

Change committed as 275159

[Peter Kasting, 2014-06-05 17:38:47]

LGTM