Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import open windows and tabs from legacy Brave #981

Merged
merged 1 commit into from
Nov 30, 2018
Merged

Conversation

garrettr
Copy link
Contributor

@garrettr garrettr commented Nov 29, 2018

Partial resolution brave/brave-browser#998. Only supports importing windows and tabs from legacy Brave, which is sufficient to support the goal of auto-migrating users from Muon to brave-core in 0.57.x.

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests) on
    • Windows
    • macOS
    • Linux
  • Ran git rebase master (if needed).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

  1. Create a test set of open windows/tabs in Muon.
    • There should be at least 1 window with at least 1 tab
    • There should be at least 1 pinned tab.
    • You should select different tabs in each window (to verify the selected tabs are maintained across import).
  2. In Muon's preferences, verify the setting "Brave starts with" is set to "My windows / tabs from last time" (the default).
  3. Quit Muon.

1. Manual import

Clear the Brave Development user data directory, then:

  1. npm start
  2. Once browser comes up, go to chrome://settings/importData
  3. In the drop down, pick Brave (old)
  4. Verify a new option Open windows and tabs is present
  5. Select only Open windows and tabs
  6. Click Import
  7. You should see the open windows and tabs open in the background (behind the window with the import dialog) and start to load.

If you select all import options, instead of only Open windows and tabs, all of the tabs should appear identical or nearly identical to what was visible when you quit Muon (because all of the session data was also imported).

2. Automatic/first-run import

Clear the Brave Development user data directory, then run the Brave binary with the --upgrade-from-muon flag, e.g. ./src/out/Debug/Brave\ Browser\ Development.app/Contents/MacOS/Brave\ Browser\ Development --enable-logging --v=0 --disable-brave-update --upgrade-from-muon.

Once loaded, this should be nearly indistinguishable from the previous Muon session. The first run experience window should remain in the foreground of all of the imported windows.

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

Copy link
Member

@darkdh darkdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me. can you squash three commits into one?
The patch issue is addressed in efe72e2#diff-6b0d3600d50944ab08b6145722eaa7c2

@garrettr
Copy link
Contributor Author

@darkdh Squashed

Copy link
Member

@darkdh darkdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++, also did a local test and it works as expected

@garrettr
Copy link
Contributor Author

I have tested and confirmed that this PR builds and works as expected on macOS and Linux, and I'm waiting on a Windows build so I can test there too. Once I've verified this is working on all platforms, I'll merge and request uplift to 0.57.x.

@garrettr
Copy link
Contributor Author

Confirmed this PR builds and manual test plan passes on Windows.

@garrettr garrettr merged commit 62dbbb9 into master Nov 30, 2018
@garrettr garrettr deleted the issue/998 branch November 30, 2018 03:33
@bbondy
Copy link
Member

bbondy commented Nov 30, 2018

🎉

@bsclifton
Copy link
Member

Great job, @garrettr! 😄 👍

bsclifton pushed a commit that referenced this pull request Nov 30, 2018
Import open windows and tabs from legacy Brave
@bsclifton
Copy link
Member

bsclifton commented Nov 30, 2018

master 62dbbb9
0.59.x 705ef5d
0.58.x d34758d
0.57.x f8ccd8f

bsclifton pushed a commit that referenced this pull request Nov 30, 2018
Import open windows and tabs from legacy Brave
bsclifton pushed a commit that referenced this pull request Nov 30, 2018
Import open windows and tabs from legacy Brave
@bsclifton
Copy link
Member

Uplifted to 0.57.x 👍
cc: @kjozwiak @srirambv @rebron

@kjozwiak
Copy link
Member

Awesome, thanks for getting this done and merged @bsclifton @garrettr 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants