-
Notifications
You must be signed in to change notification settings - Fork 936
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
Guard ChromeActivity.getTabModelSelector with areTabModelsInitialized #26724
Conversation
…ed() Fixes crash reported from GP dev console. Fixes brave/brave-browser#42319
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Added just a minor suggestion. Changes look good!
if (!chromeActivity.areTabModelsInitialized()) { | ||
return ProfileManager.getLastUsedRegularProfile(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because both return ProfileManager.getLastUsedRegularProfile();
I would unify this if statement with the one above by simply changing the condition into if (chromeActivity == null || !chromeActivity.areTabModelsInitialized())
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, fixed at a2a11d8
ChromeTabbedActivity activity = BraveRewardsHelper.getChromeTabbedActivity(); | ||
if (activity == null | ||
|| !activity.areTabModelsInitialized() | ||
|| activity.getTabModelSelector() == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please also change getTabModelSelector
(it's deprecated) to getTabModelSelectorSupplier
as
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, changed at 3ba5886
…ctor, codereview notice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++
Released in v1.75.53 |
This PR fixes crash reported from GP dev console, also fixes other similar palces.
Fixes brave/brave-browser#42319
Resolves brave/brave-browser#42319
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
There is no one, crash was reported from GP dev console.