Import from Chrome #49
Conversation
garrettr
force-pushed
the
chrome-importer
branch
from
297dacd
to
625256f
Compare
- Fix incorrect propagation of is_in_toolbar flag - Only add ImportedBookmarkEntry for folders if they're empty Importing bookmarks from Chrome now conforms to the behavior described in the comment for ProfileWriter::AddBookmarks: https://cs.chromium.org/chromium/src/chrome/browser/importer/profile_writer.h?l=56-73&rcl=219101ecc0143651567d49340e80fb5b3e756b40
garrettr
force-pushed
the
chrome-importer
branch
from
d75374d
to
20ca9be
Compare
|
Rebased after moving the groundwork for the tests into #57. Once my local rebuild is done, I will check that this is ready for review by @darkdh, e.g. it builds on macOS and it passes both automated and manual testing. I will also test on Windows and Linux. Additional data import types, e.g. cookies and saved passwords, will be implemented in another PR. |
|
This is ready for review on macOS. To test:
|
There was a problem hiding this comment.
we should prompt users to close Chrome because History sqlite file will get locked while Chrome is opened. And when that happened, nothing will be imported.
reference FirefoxProfileLock
and we can check SingletonLock under user folder on Mac and Linux
and lockfile under user folder on Windows
you can do this as follow up
@darkdh I filed https://github.com/brave/brave/issues/103. Is this PR ok to merge otherwise? |
| @@ -0,0 +1,73 @@ | |||
| // Copyright (c) 2016 GitHub, Inc. | |||
There was a problem hiding this comment.
I think we should change the license to MPL-2.0
| class ListValue; | ||
| } | ||
|
|
||
| base::FilePath GetChromeUserDataFolder(); |
There was a problem hiding this comment.
we can add chromium and canary support
| } | ||
| } | ||
|
|
||
| void ChromeImporter::LoadFaviconData( |
There was a problem hiding this comment.
do you mind back porting the fix to https://github.com/brave/muon/blob/a43303869aead146332e080518ed45df7f0de62d/brave/utility/importer/chrome_importer.cc#L208 ?
There was a problem hiding this comment.
Filed brave/muon#539
| // Folders are added implicitly on adding children, so we only | ||
| // explicitly add empty folders. | ||
| const base::ListValue* children; | ||
| if (dict->GetList("children", &children) && children->empty()) { |
There was a problem hiding this comment.
don't back port this to muon because muon uses different profile writer
|
@darkdh I changed all of the .cc/.h license headers to MPL and ported the additional support for importing from Chromium and Chrome Canary from Muon. Re-review? |
|
@darkdh Why not just use the "squash and merge" option in the "Merge pull request" dropdown? |
|
For future reference I prefer to not squash and keep commit history more granular, unless it's a messy log, then move to squash and merge. Thanks. Nice work on getting this landed! Thanks for reviewing Anthony! |
remove more platform specific code
add support for getting user model and schema data from non-filesyste…
The goal of this PR is to port the "Import from Chrome" feature from Brave/Muon to the Chromium fork.
I'm sharing my work-in-progress branch to facilitate review and discussion of my approach so far (per discussion with @bbondy). I started by simplifying the feature to only support importing history and bookmarks from Chrome (per suggestion from @darkdh). I have successfully manually tested importing history and bookmarks from Chrome on macOS with this PR.
Remaining work: