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

Fixed crash when dragging entered into sidebar web panel #8649

Merged
merged 1 commit into from
Apr 27, 2021

Conversation

simonhong
Copy link
Member

@simonhong simonhong commented Apr 27, 2021

fix brave/brave-browser#15505

Set dummy WebContentsDelegate to sidebar WebContents to fix crash.
Real delegate implementation will be done as a separate task.

Resolves

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

@simonhong simonhong self-assigned this Apr 27, 2021
@simonhong simonhong marked this pull request as ready for review April 27, 2021 01:53
@simonhong simonhong requested a review from emerick April 27, 2021 01:55
Copy link
Contributor

@emerick emerick left a comment

Choose a reason for hiding this comment

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

LGTM

@simonhong simonhong added CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux labels Apr 27, 2021
fix brave/brave-browser#15505
Set dummy WebContentsDelegate to sidebar WebContents.
Real delegate implementation will be done as a separate task.
@simonhong
Copy link
Member Author

Merged because only unrelated one test BraveAdsBrowserTest/BraveAdsUpgradeBrowserTest was failed.

@simonhong simonhong merged commit 343b2db into master Apr 27, 2021
@simonhong simonhong deleted the fix_sidebar_crash branch April 27, 2021 14:04
@stephendonner
Copy link
Contributor

Verified PASSED using

Brave 1.26.11 Chromium: 90.0.4430.93 (Official Build) nightly (x86_64)
Revision 4df112c29cfe9a2c69b14195c0275faed4e997a7-refs/branch-heads/4430@{#1348}
OS macOS Version 11.3.1 (Build 20E241)

Was able to very easily reproduce this using earlier nightly builds; I followed the steps from brave/brave-browser#15505, and observed no crashes:

  1. enabled sidebar feature via brave://flags and re-launched
  2. loaded brave.com in a new tab
  3. clicked sidebar's Brave Together panel
  4. dragged brave.com tab's site info (security icon in omnibox) into sidebar web panel

sidebar-crash

kylehickinson pushed a commit that referenced this pull request Jan 17, 2024
* Update performance of `SelectAccountTokenStore`; build account sections for changes instead of computed property

* Fix for case where balance is 0 after being cached as non-zero does not update cache, don't lose cached balances after failed response.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crashed when dragging enters into sidebar web panel
3 participants