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.

Sign up for GitHub

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

Jump to bottom

Fix relaunch fail after update on mac #1041

Merged
merged 2 commits into from
Dec 7, 2018
Merged

Fix relaunch fail after update on mac #1041

merged 2 commits into from
Dec 7, 2018

Conversation

simonhong
Copy link
Member

@simonhong simonhong commented Dec 7, 2018 •
edited
Loading

First commit is cleanup.
Second commit is fix for relaunch fail after update.
Sparkle framework's relaunch api is used instead of using chromium's relaunch api.

This change will not affect other platforms(linux and win).

Fix brave/brave-browser#977
Fix brave/brave-browser#2339
Fix brave/brave-browser#1787
Fix brave/brave-browser#1745

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. Modify version in source tree to lower than released stable version for update test
    Ex, change 55.1 in package.json and chrome/VERSION.
  2. Build master release mode
  3. Start browser with update enabled mode
    yarn start Release --enable_brave_update
  4. Open help page and waiting to finish update and push relaunch button
  5. Check relaunch is done

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

@simonhong
Prevent registerWithSparkle is called twice
59493e9 
It could be called twice when browser is restarted by relaunch not by
quit & launch. However, Calling this twice might not cause problem.
This commit just make calling once exactly.
@simonhong
Use sparkle framework's relaunch api for browser relaunch after update
88e4277 
Chromium's relaunch api seems not compatible with sparkle framework.
This new relaunch api is only used by relaunch button in about page.
@simonhong simonhong self-assigned this Dec 7, 2018
darkdh
darkdh approved these changes Dec 7, 2018
View reviewed changes
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.

++

@simonhong simonhong merged commit 383fb12 into master Dec 7, 2018
@simonhong simonhong deleted the fix_relaunch_fail_after_update_on_mac branch December 7, 2018 23:07
@simonhong
Copy link
Member Author

master(383fb12)

@simonhong
Copy link
Member Author

@rebron @kjozwiak @srirambv How about uplifting this fix?

@bbondy
Copy link
Member

bbondy commented Dec 8, 2018

Changed request to 0.58.x beta since 0.57.x is already released.

@kjozwiak
Copy link
Member

kjozwiak commented Dec 9, 2018

@simonhong uplift request to 0.58.x approved after deliberating with @srirambv & @rebron 👍Please make sure that all the appropriate labels are added/removed and the appropriate issue moved to the correct milestone.

simonhong added a commit that referenced this pull request Dec 10, 2018
@simonhong
Merge pull request #1041 from brave/fix_relaunch_fail_after_update_on…
222e953 
…_mac

Fix relaunch fail after update on mac
simonhong added a commit that referenced this pull request Dec 10, 2018
@simonhong
Merge pull request #1041 from brave/fix_relaunch_fail_after_update_on…
a71b91d 
…_mac

Fix relaunch fail after update on mac
@simonhong
Copy link
Member Author

simonhong commented Dec 10, 2018 •
edited
Loading

master(383fb12)
0.59.x(222e953)
0.58.x(a71b91d)

@simonhong simonhong mentioned this pull request Dec 11, 2018
Merged
18 tasks
@bbondy bbondy added this to the 0.58.x - Release milestone Jan 14, 2019
@justanotheranonymoususer justanotheranonymoususer mentioned this pull request Sep 23, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darkdh darkdh darkdh approved these changes

@bbondy bbondy Awaiting requested review from bbondy

Assignees

@simonhong simonhong

Labels
None yet
Projects
None yet
Milestone
0.58.x - Release
4 participants
@simonhong @bbondy @kjozwiak @darkdh