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

Fixes a crash on calling TemplateUrlServiceFactory when main Activity is stopping #21328

Merged
merged 1 commit into from
Dec 11, 2023
Merged

Fixes a crash on calling TemplateUrlServiceFactory when main Activity is stopping #21328

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 3 additions & 13 deletions android/java/org/chromium/chrome/browser/app/BraveActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@
import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.browser.tab.TabSelectionType;
import org.chromium.chrome.browser.tabmodel.TabModel;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tabmodel.TabModelUtils;
import org.chromium.chrome.browser.toolbar.bottom.BottomToolbarConfiguration;
import org.chromium.chrome.browser.toolbar.top.BraveToolbarLayoutImpl;
Expand Down Expand Up @@ -306,12 +307,6 @@ public void onResumeWithNative() {
BraveVpnNativeWorker.getInstance().addObserver(this);
BraveVpnUtils.reportBackgroundUsageP3A();
}
Profile profile = getCurrentTabModel().getProfile();
if (profile != null) {
// Set proper active DSE whenever brave returns to foreground.
// If active tab is private, set private DSE as an active DSE.
BraveSearchEngineUtils.updateActiveDSE(profile);
}

// The check on mNativeInitialized is mostly to ensure that mojo
// services for wallet are initialized.
Expand Down Expand Up @@ -342,12 +337,6 @@ public void onPauseWithNative() {
if (BraveVpnUtils.isVpnFeatureSupported(BraveActivity.this)) {
BraveVpnNativeWorker.getInstance().removeObserver(this);
}
Profile profile = getCurrentTabModel().getProfile();
if (profile != null && profile.isOffTheRecord()) {
// Set normal DSE as an active DSE when brave goes in background
// because currently set DSE is used by outside of brave(ex, brave search widget).
BraveSearchEngineUtils.updateActiveDSE(profile);
Copy link
Contributor

Choose a reason for hiding this comment

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

@SergeyZhukovsky are we okay removing the change here ?

Copy link
Member Author

@SergeyZhukovsky SergeyZhukovsky Dec 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I verified such steps:

  1. Set standard tab SE to Brave.
  2. Set private tab SE to Qwant.
  3. Made some search on both and made sure they are correct.
  4. Switched to Private tab, closed Brave.
  5. Added a search widget.
  6. Tried to search via widget and made sure Brave SE is used in a standard tab is used for that.

Do you have other steps or do you know why we added the above?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't see any reason why it's there, but I'll keep an eye what happens once the change hits users.

Copy link
Contributor

Choose a reason for hiding this comment

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

can you verify the change with quick action search widget we have ?
Steps :

  1. add widget on then home screen
  2. open brave app and change search engine from Brave to something else.
  3. Minimize the app and check widget, if it shows the write search engine on the url bar.

If above looks good, we are good to go ahead with the change.

Copy link
Member Author

@SergeyZhukovsky SergeyZhukovsky Dec 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Screen_recording_20231211_152350.webm

}
super.onPauseWithNative();
}

Expand Down Expand Up @@ -803,7 +792,6 @@ public void initializeState() {
setLoadedFeed(false);
setComesFromNewTab(false);
setNewsItemsFeedCards(null);
BraveSearchEngineUtils.initializeBraveSearchEngineStates(getTabModelSelector());
Copy link
Member Author

Choose a reason for hiding this comment

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

I moved it to finishNativeInitialization as it does native calls inside.

Intent intent = getIntent();
if (intent != null && intent.getBooleanExtra(Utils.RESTART_WALLET_ACTIVITY, false)) {
openBraveWallet(false,
Expand Down Expand Up @@ -938,6 +926,8 @@ public void maybeSolveAdaptiveCaptcha() {
@Override
public void finishNativeInitialization() {
super.finishNativeInitialization();
BraveSearchEngineUtils.initializeBraveSearchEngineStates(
(TabModelSelector) getTabModelSelectorSupplier().get());
BraveVpnNativeWorker.getInstance().reloadPurchasedState();

BraveHelper.maybeMigrateSettings();
Expand Down