Rest of Muon import fails after bookmarks import fails

[garrettr]

Description

Separate issue for "Bookmark import fails scenario" from #2438.

Original description

(reported by several folks on Reddit; need help finding an example session with bad bookmarks)

Steps to Reproduce

1. Have a specific setup / bookmarks on Muon.
2. Exit Muon and Launch Brave Core with a fresh profile
3. Use chrome://settings/importData to import your Muon profile
4. If you have "corrupt" bookmarks, the import will fail

Actual result:

Steps after the bookmark import (Passwords, Cookies, Stats, Windows/Tabs, Brave Payments) are not executed.

Expected result:

As many bookmarks as possible should be imported. If bookmarks import fails, the importer should still continue and attempt to import the remaining selected data types.

Reproduces how often:

Intermittent issue. We need one or more examples of a corrupt session-store-1 that can be used to reproduce.

Brave version (brave://version info)

Reproducible on current release:

• Does it reproduce on brave-browser dev/beta builds?

Website problems only:

• Does the issue resolve itself when disabling Brave Shields?
• Is the issue reproducible on the latest version of Chrome?

Additional Information

[garrettr]

Based on a review of the code, there are several places in the result of base::Value::FindKeyOfType is dereferenced without first checking for a nullptr. If a bookmarks-related data structure in session-store-1 does not have the expected keys, this could cause the utility process to crash, which would prevent the remaining data types from being imported, similar to #2513.

[garrettr]

Here's an artificial example of a session-store-1 that seems to trigger this issue: session-store-1.zip. To create it, I:

1. Started Muon with a fresh profile.
2. Added a single bookmark to the Bookmarks Toolbar for https://brave.com
3. Quit Muon
4. Manually edited session-store-1 to remove the title key from the object corresponding to the "https://brave.com" bookmark in the "bookmarks" object.

This may not be the true, or the only true, cause of this issue. I do not know if or how it would be possible for a naturally occurring session-store-1 to be malformed in a way that would trigger one of these nullptr dereferences; however, I do think it would be straightforward and prudent to handle these cases better.

It would still be useful to see one or more real-world examples of a session-store-1 that causes this or similar failures in the importer.

[GeetaSarvadnya]

Verificatio passed on

Brave	0.58.15 Chromium: 71.0.3578.98 (Official Build) (64-bit)
Revision	15234034d19b85dcd9a03b164ae89d04145d8368-refs/branch-heads/3578@{#897}
OS	Windows

• Verified using session-store-1 from Rest of Muon import fails after bookmarks import fails #2517 (comment) - Import bookmarks fails ( which is expected as session-store-1 is corrupted)
• Verified muon data import into bc 0.58.15 in fresh profile. Verification is PASSED and i am able to see below data on bc after manual import brave://settings/importData
  -Bookmarks ( Imported all the 3 browser (IE, FF and Chrome) bookmarks into 0.25.303)
  -Stats
  -Open window tabs ( I have 3 sites (nytimes, cnn and cnet) opened in 0.25.303)
  -Rewards data ( Rewards auto enables/ Min page time/Min visits/Rewards balance/Monthly budget/Pinned sites and total monthly pinned amount)
  -Saved passwords
  -Cookies and History

Verified passed with

Brave	0.58.15 Chromium: 71.0.3578.98 (Official Build) (64-bit)
Revision	15234034d19b85dcd9a03b164ae89d04145d8368-refs/branch-heads/3578@{#897}
OS	Mac OS X

• Verified using session-store-1 from Rest of Muon import fails after bookmarks import fails #2517 (comment)
• Additionally, I added my own passwords, cookies, payments data, additional history & open tabs to that session-store-1 file.
• Bookmark import failed (as expected) but History, Passwords, Cookies, Stats, Payments, open windows/tabs were all imported (note - had to allow keychain access for Passwords and Cookies to be imported).