Improve Firefox importer to handle all Firefox profiles.

Currently there is a Firefox importer that handles only one version of
the Firefox profile. Most of the information about bookmarks and history
is stored in places.sqlite. When the schema of this file changes
significantly the importer stops working. After fixing the SQL queries
to account for those changes, the importer stops working for the earlier
schema versions.

This CL aims to:
1) Introduce FirefoxPlacesFactory that detects places.sqlite schema
version and creates an appropriate handler. This allows the importer to
work for ALL versions of Firefox profiles. It also makes future updates
easier.
2) Refactor firefox_importer.cc a little bit. This file is long and
does all the work that could be split into a few, more logical parts.

BUG=592239

[kszatan, 2016-10-27 11:51:45]

Description was changed from

==========
Currently there is a Firefox importer that handles only one version of
the Firefox profile. Most of the information about bookmarks and history
is stored in places.sqlite. When the schema of this file changes
significantly the importer stops working. After fixing the SQL queries
to account for those changes, the importer stops working for the earlier
schema versions.

This CL aims to:
1) Introduce FirefoxPlacesFactory that detects places.sqlite schema
version and creates an appropriate handler. This allows the importer to
work for ALL versions of Firefox profiles. It also makes future updates
easier.
2) Refactor firefox_importer.cc a little bit. This file is long and
does all the work that could be split into a few, more logical parts.

BUG=
==========

to

==========
Currently there is a Firefox importer that handles only one version of
the Firefox profile. Most of the information about bookmarks and history
is stored in places.sqlite. When the schema of this file changes
significantly the importer stops working. After fixing the SQL queries
to account for those changes, the importer stops working for the earlier
schema versions.

This CL aims to:
1) Introduce FirefoxPlacesFactory that detects places.sqlite schema
version and creates an appropriate handler. This allows the importer to
work for ALL versions of Firefox profiles. It also makes future updates
easier.
2) Refactor firefox_importer.cc a little bit. This file is long and
does all the work that could be split into a few, more logical parts.

BUG=
==========

[kszatan, 2016-10-27 11:51:45]

kszatan@opera.com changed reviewers:
+ isherman@chromium.org

[kszatan, 2016-12-05 09:36:02]

Description was changed from

==========
Currently there is a Firefox importer that handles only one version of
the Firefox profile. Most of the information about bookmarks and history
is stored in places.sqlite. When the schema of this file changes
significantly the importer stops working. After fixing the SQL queries
to account for those changes, the importer stops working for the earlier
schema versions.

This CL aims to:
1) Introduce FirefoxPlacesFactory that detects places.sqlite schema
version and creates an appropriate handler. This allows the importer to
work for ALL versions of Firefox profiles. It also makes future updates
easier.
2) Refactor firefox_importer.cc a little bit. This file is long and
does all the work that could be split into a few, more logical parts.

BUG=
==========

to

==========
Currently there is a Firefox importer that handles only one version of
the Firefox profile. Most of the information about bookmarks and history
is stored in places.sqlite. When the schema of this file changes
significantly the importer stops working. After fixing the SQL queries
to account for those changes, the importer stops working for the earlier
schema versions.

This CL aims to:
1) Introduce FirefoxPlacesFactory that detects places.sqlite schema
version and creates an appropriate handler. This allows the importer to
work for ALL versions of Firefox profiles. It also makes future updates
easier.
2) Refactor firefox_importer.cc a little bit. This file is long and
does all the work that could be split into a few, more logical parts.

BUG=592239
==========

[Ilya Sherman, 2016-12-09 00:52:29]

Hmm, is there much value in continuing to support older Firefox schemas?  I'm
not convinced that it's worth the extra code complexity.  According to [
http://caniuse.com/usage-table ], the vast majority of Firefox users should be
running a version of Firefox that has the latest schema.

[kszatan, 2016-12-19 11:16:17]

The main problem that this CL should solve is the situation when FF profile's
schema changes. If the code depends on a particular schema version we'll get
complaints from users trying to import a profile from the newest FF version.
After we update the code to use the newest schema we'll get complaints from
users using older versions.
It's reasonable to assume that we need to only handle two latest schema
versions. I'll try to come up with a lighter version of this patch that will
help make this transition painless.

[Ilya Sherman, 2016-12-19 21:36:03]

On 2016/12/19 11:16:17, kszatan wrote:
> The main problem that this CL should solve is the situation when FF profile's
> schema changes. If the code depends on a particular schema version we'll get
> complaints from users trying to import a profile from the newest FF version.
> After we update the code to use the newest schema we'll get complaints from
> users using older versions.
> It's reasonable to assume that we need to only handle two latest schema
> versions. I'll try to come up with a lighter version of this patch that will
> help make this transition painless.

I suspect that it's fine to only handle the latest schema, TBH.  It usually
takes us O(months) to notice that the schema has changed, and update to it.  By
then, most users are on the newest schema; and for users who aren't, we can
either (a) ask them to upgrade, or (b) export their bookmarks from Firefox and
import them from a file in Chrome.  So, I don't think it's worth having anything
more than the simplest approach within the Chromium code.  That said, if there's
a very simple and elegant solution for supporting the two latest schema
versions, I'm not 100% opposed to such a change.  I simply think that code
simplicity is much more valuable than supporting older schema versions here,
given the availability of workarounds, and the minimal degree to which this code
is owned/maintained within Chromium.

[Ilya Sherman, 2016-12-19 21:36:52]

On 2016/12/19 21:36:03, Ilya Sherman wrote:
> On 2016/12/19 11:16:17, kszatan wrote:
> > The main problem that this CL should solve is the situation when FF
profile's
> > schema changes. If the code depends on a particular schema version we'll get
> > complaints from users trying to import a profile from the newest FF version.
> > After we update the code to use the newest schema we'll get complaints from
> > users using older versions.
> > It's reasonable to assume that we need to only handle two latest schema
> > versions. I'll try to come up with a lighter version of this patch that will
> > help make this transition painless.
> 
> I suspect that it's fine to only handle the latest schema, TBH.  It usually
> takes us O(months) to notice that the schema has changed, and update to it. 
By
> then, most users are on the newest schema; and for users who aren't, we can
> either (a) ask them to upgrade, or (b) export their bookmarks from Firefox and
> import them from a file in Chrome.  So, I don't think it's worth having
anything
> more than the simplest approach within the Chromium code.  That said, if
there's
> a very simple and elegant solution for supporting the two latest schema
> versions, I'm not 100% opposed to such a change.  I simply think that code
> simplicity is much more valuable than supporting older schema versions here,
> given the availability of workarounds, and the minimal degree to which this
code
> is owned/maintained within Chromium.

Oh, and: Thanks very much for updating the import code to support the most
recent schema version, and for looking at this in general! =)

[kszatan, 2016-12-20 08:55:25]

On 2016/12/19 21:36:03, Ilya Sherman (Away De.29-Ja.4) wrote:
> I suspect that it's fine to only handle the latest schema, TBH.  It usually
> takes us O(months) to notice that the schema has changed, and update to it. 
By
> then, most users are on the newest schema;
Ok, in that case, I'll focus on re-enabling importer browser tests and fixing
importing of passwords. I'll do that in another CL.

[Ilya Sherman, 2016-12-21 03:35:45]

On 2016/12/20 08:55:25, kszatan wrote:
> On 2016/12/19 21:36:03, Ilya Sherman (Away De.29-Ja.4) wrote:
> > I suspect that it's fine to only handle the latest schema, TBH.  It usually
> > takes us O(months) to notice that the schema has changed, and update to it. 
> By
> > then, most users are on the newest schema;
> Ok, in that case, I'll focus on re-enabling importer browser tests and fixing
> importing of passwords. I'll do that in another CL.

Sounds great -- thank you =)