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

Rewards button icon pre-opt-in gets correct scaling #3830

Merged
merged 1 commit into from
Oct 31, 2019

Conversation

petemill
Copy link
Member

@petemill petemill commented Oct 30, 2019

Sets the scale of the image compared to the size we want to display it, so that the underlying Draw function can work out if the image needs to be re-sized or not. Previously this was only happening (coincidentally) when the display was not using a scaling factor of 1 since this would not match the default scaling factor of the gfx::Image. Resize is done here: https://github.com/chromium/chromium/blob/0aa1854a269704ce0dd45641cfb9d440666e54bc/chrome/browser/ui/extensions/icon_with_badge_image_source.cc#L147-L148

Fix: brave/brave-browser#6663

This should be uplifted to wherever #3698 is uplifted to (currently 0.71.x).

Submitter Checklist:

Test Plan:

On brave/brave-browser#6663

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

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@petemill petemill force-pushed the fix-6663-bat-icon-scale branch 2 times, most recently from 0fbbb9d to e3f343a Compare October 31, 2019 00:46
@petemill
Copy link
Member Author

Copy link
Collaborator

@mkarolin mkarolin left a comment

Choose a reason for hiding this comment

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

++

@simonhong
Copy link
Member

simonhong commented Oct 31, 2019

This works on Windows debug build 👍

@simonhong
Copy link
Member

simonhong commented Oct 31, 2019

I'm not sure this is related with this PR but crash happened when I click rewards button.
I can see this crash only on linux.
=> I can see this crash w/o this PR. Maybe another change caused this crash.

Received signal 11 SEGV_MAPERR 000000000000
#0 0x7f0f0bead72f base::debug::CollectStackTrace()
#1 0x7f0f0bbe4d1d base::debug::StackTrace::StackTrace()
#2 0x7f0f0bbe4cd8 base::debug::StackTrace::StackTrace()
#3 0x7f0f0bead1de base::debug::(anonymous namespace)::StackDumpSignalHandler()
#4 0x7f0ed5908890 <unknown>
#5 0x55f762ff8e78 ExtensionActionViewController::ExecuteAction()
#6 0x55f762ff8dcd ExtensionActionViewController::ExecuteAction()
#7 0x55f75e000b67 BraveActionsContainer::AddAction()
#8 0x55f75e001adb BraveActionsContainer::OnExtensionLoaded()
#9 0x55f75ce3158c extensions::ExtensionRegistry::TriggerOnLoaded()
#10 0x55f75ce2a3ea extensions::ExtensionRegistrar::ActivateExtension()
#11 0x55f75ce2a2c3 extensions::ExtensionRegistrar::AddNewExtension()
#12 0x55f75ce2937c extensions::ExtensionRegistrar::AddExtension()
#13 0x55f7617ca2bd extensions::ExtensionService::AddExtension()
#14 0x55f7617ca7c9 extensions::ExtensionService::AddComponentExtension()
#15 0x55f75e68e6b7 extensions::BraveExtensionService::AddComponentExtension()
#16 0x55f76173a75c extensions::ComponentLoader::Load()
#17 0x55f76173ad76 extensions::ComponentLoader::Add()
#18 0x55f76173abd2 extensions::ComponentLoader::Add()
#19 0x55f76173aaf8 extensions::ComponentLoader::Add()
#20 0x55f75e68a69a extensions::BraveComponentLoader::AddRewardsExtension()
#21 0x55f75e001a6a BraveActionsContainer::OnRewardsStubButtonClicked()
#22 0x55f75e014377 BraveRewardsActionStubView::ButtonPressed()
#23 0x7f0ef576da9b views::Button::NotifyClick()
#24 0x7f0ef576b89c views::Button::DefaultButtonControllerDelegate::NotifyClick()
#25 0x7f0ef5772027 views::ButtonController::OnMouseReleased()
#26 0x7f0ef576ca9f views::Button::OnMouseReleased()
#27 0x7f0ef58e58b4 views::View::ProcessMouseReleased()
#28 0x7f0ef58e53b9 views::View::OnMouseEvent()
#29 0x7f0efa9e330b ui::EventHandler::OnEvent()
#30 0x7f0efa9f593c ui::ScopedTargetHandler::OnEvent()
#31 0x7f0efa9e0284 ui::EventDispatcher::DispatchEvent()
#32 0x7f0efa9dfbdb ui::EventDispatcher::ProcessEvent()
#33 0x7f0efa9df83c ui::EventDispatcherDelegate::DispatchEventToTarget()
#34 0x7f0efa9df665 ui::EventDispatcherDelegate::DispatchEvent()
#35 0x7f0ef5905f29 views::internal::RootView::OnMouseReleased()
#36 0x7f0ef590f21d views::Widget::OnMouseEvent()
#37 0x7f0ef5964e81 views::DesktopNativeWidgetAura::OnMouseEvent()
#38 0x7f0efa9e330b ui::EventHandler::OnEvent()
#39 0x7f0efa9e0284 ui::EventDispatcher::DispatchEvent()
#40 0x7f0efa9dfbdb ui::EventDispatcher::ProcessEvent()
#41 0x7f0efa9df83c ui::EventDispatcherDelegate::DispatchEventToTarget()
#42 0x7f0efa9df665 ui::EventDispatcherDelegate::DispatchEvent()
#43 0x7f0efa9e4a60 ui::EventProcessor::OnEventFromSource()
#44 0x7f0efa9e4d04 ui::EventProcessor::OnEventFromSource()
#45 0x7f0efa9e6923 ui::EventSource::DeliverEventToSink()
#46 0x7f0efa9e66a8 ui::EventSource::SendEventToSinkFromRewriter()
#47 0x7f0efa9e63e1 ui::EventSource::SendEventToSink()
#48 0x7f0ef597c4c0 views::DesktopWindowTreeHostX11::DispatchMouseEvent()
#49 0x7f0ef597cd03 views::DesktopWindowTreeHostX11::DispatchEvent()

Copy link
Member

@simonhong simonhong left a comment

Choose a reason for hiding this comment

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

Confirmed on linux (ubuntu) and Win10 that rewards icon is displayed with proper scaling.
I can see crash on linux when rewards button is clicked, but it seems another issue.

@petemill
Copy link
Member Author

Thanks @simonhong , I'll check out the crash

@petemill petemill merged commit 2ffccbb into master Oct 31, 2019
@petemill petemill deleted the fix-6663-bat-icon-scale branch October 31, 2019 05:24
@petemill
Copy link
Member Author

FYI @simonhong 's crash was caused by setting the "Extensions Toolbar Menu" flag to true. That is a known issue and the flag is disabled by default.

@petemill petemill added this to the 0.73.x - Nightly milestone Oct 31, 2019
petemill added a commit that referenced this pull request Oct 31, 2019
Rewards button icon pre-opt-in gets correct scaling
petemill added a commit that referenced this pull request Oct 31, 2019
Rewards button icon pre-opt-in gets correct scaling
bsclifton added a commit that referenced this pull request Nov 1, 2019
Uplift to 0.72.x: Rewards button icon pre-opt-in gets correct scaling (#3830)
bsclifton added a commit that referenced this pull request Nov 1, 2019
Uplift to 0.71.x: Rewards button icon pre-opt-in gets correct scaling (#3830)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BAT icon in URL bar is misplaced when not rewards user and screen is 100% scaling
3 participants