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

Unexpected behavior after relaunch (macOS) #25576

Open
justanotheranonymoususer opened this issue Sep 23, 2022 · 6 comments
Open

Unexpected behavior after relaunch (macOS) #25576

justanotheranonymoususer opened this issue Sep 23, 2022 · 6 comments
Assignees
Labels

Comments

@justanotheranonymoususer

Description

There's an issue I saw several times on Mac with browser relaunch. If an update is available, and relaunch is triggered from a non-update-related flow, weird stuff is happening.

Steps to Reproduce

It's tricky to reproduce because you need to have a browser with a pending update. I'm not sure that the steps are precise but I believe it's something similar.

  1. Launch Brave on Mac and wait for an update to be available and pending.
  2. Go to chrome://flags.
  3. Change a flag, you'll see a button to relaunch the browser.
  4. Click the button to relaunch the browser.

Actual result:

Things break. One time the browser crashed. Other time it launched but was just a blank window, without tabs or URL bar or anything at all.

Expected result:

Proper relaunch.

Reproduces how often:

Intermittent issue.

Brave version (brave://version info)

Latest Mac and latest Brave.

Version/Channel Information:

Didn't try but pretty sure it's not channel-specific.

Other Additional Information:

I'm pretty sure it's related to these changes:

I tried to contribute a fix but gave up. What I figured is that there are two ways to relaunch a browser: with Chromium functions and with code from Sparkle. The Chromium functions only work when no update is pending, and the Sparkle function only works when an update is pending. But Brave doesn't check whether an update is pending. Instead, it tries to guess, e.g. if you're on chrome://flag, chances are no update is pending. This assumption can often be incorrect.

What I tried to do is changing Sparkle's relaunch function to handle the case of no pending update and just return false instead of throwing an assert. Then, this code can be used in Brave (pseudo code):

bool relaunched = [sparkle_glue relaunch];
if (!relaunched) {
  chrome::relaunch();
}

But as I said, changing Sparkle and the Objective C code was too much for me.

  • Does the issue resolve itself when disabling Brave Shields? Unrelated
  • Does the issue resolve itself when disabling Brave Rewards? Unrelated
  • Is the issue reproducible on the latest version of Chrome? No, Chrome doesn't use Sparkle on Mac

Miscellaneous Information:

@rebron
Copy link
Collaborator

rebron commented Sep 23, 2022

Hi @justanotheranonymoususer Can you post the version you're using from here brave://version
example below. Select and copy through User Agent and paste. Also, what macOS version are you using.

Brave 1.45.68 Chromium: 106.0.5249.55 (Official Build) beta (arm64)
Revision 4d5f098fca6ab7f4b6b7c240be3d9593c2357709-refs/branch-heads/5249@{#531}
OS macOS Version 13.0 (Build 22A5342f)
JavaScript V8 10.6.194.11
User Agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/106.0.0.0 Safari/537.36

@simonhong
Copy link
Member

simonhong commented Sep 23, 2022

@justanotheranonymoususer Thanks for reporting!
I didn't check yet with your scenario but I think it could be possible.
I'll take a look soon.

I think all macOS relaunch code path should be like below

if ([sparkle_glue recentStatus] == kAutoupdateInstalled) {
  [sparkle_glue relaunch];
} else {
  chrome::relaunch();
}

@simonhong simonhong self-assigned this Sep 23, 2022
@justanotheranonymoususer
Copy link
Author

I think all macOS relaunch code path should be like below

if ([sparkle_glue recentStatus] == kAutoupdateInstalled) {
  [sparkle_glue relaunch];
} else {
  chrome::relaunch();
}

I actually thought about that, but I was afraid that the return value of recentStatus can be stale. There's the comment here ("a new update may have been installed in the background"):

https://github.com/brave/brave-core/blob/c86643363ad532e2cda935430bea1cd3c024ab91/chromium_src/chrome/browser/ui/webui/help/version_updater_mac.mm#L98-L101

@simonhong
Copy link
Member

I actually thought about that, but I was afraid that the return value of recentStatus can be stale. There's the comment here ("a new update may have been installed in the background"):

I think that comment should be updated. AFAIK, sparkle doesn't install new update if there is already installed one.
When I restart from relaunch button at relaunch modal or brave://settings/help, sometimes I can get new update from brave://settings/help right after relaunch.

@rebron rebron added the priority/P4 Planned work. We expect to get to it "soon". label Sep 27, 2022
@LaurenWags
Copy link
Member

Maybe related to #11055? cc @rebron

@simonhong
Copy link
Member

Maybe related to #11055? cc @rebron

Yes, correct. @rebron I think this could happen more frequently as we are adding more Relaunch buttons in settings.
I'm on this now.

@bsclifton bsclifton changed the title Relaunch issues on Mac Unexpected behavior after relaunch (macOS) Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: P3 Backlog
Development

Successfully merging a pull request may close this issue.

4 participants