New issue
Advanced search Search tips

Issue 838052 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Flaky-Test: BluetoothChooserBrowserTest.InvokeUi_ConnectedModal

Blocking:
issue 836116



Sign in to add a comment

BluetoothChooserBrowserTest.InvokeUi_ConnectedModal is Flaky

Project Member Reported by Findit, Apr 30 2018

Issue description

Findit has detected a flake at test BluetoothChooserBrowserTest.InvokeUi_ConnectedModal.

Culprit (70.0% confidence): https://chromium-review.googlesource.com/q/I176d4b5ed7f07fe679a2b70addcce02a09a6b5a3
Regression range: None

Analysis: https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVyvAELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCKFAWNocm9taXVtLm1lbW9yeS9MaW51eCBBU2FuIFRlc3RzIChzYW5kYm94ZWQpLzQ1OTMyL2Jyb3dzZXJfdGVzdHMvUW14MVpYUnZiM1JvUTJodmIzTmxja0p5YjNkelpYSlVaWE4wTGtsdWRtOXJaVlZwWDBOdmJtNWxZM1JsWkUxdlpHRnMMCxITTWFzdGVyRmxha2VBbmFseXNpcxgBDA

If this result was incorrect, apply the label Findit-Incorrect-Result, mark the bug as Untriaged and the component Tools>Test>Findit>Flakiness.
 
Cc: patricia...@chromium.org
The test occasionally crashes on Linux ASan
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=InvokeUi_ConnectedModal

+patricialor, author of suspected CL
PTAL and verify that this is related to your change.

Blocking: 836116
Cc: -patricia...@chromium.org
Components: UI
Owner: patricia...@chromium.org
Status: Assigned (was: Available)
Yup, this is caused by me, thanks! Looks like kBluetoothConnectedIcon / bluetooth_connected.icon (see attached for icon preview) doesn't specify a size. This is related to  Issue 836116 .

Stack trace:
[30918:30918:0429/232458.651819:FATAL:paint_vector_icon.cc(631)] Check failed: icon.reps[icon.reps_size - 1].path[0].command == CANVAS_DIMENSIONS (6 vs. 22) kBluetoothConnectedIcon has no size in its icon definition, and it seems unlikely you want to display at the default of 48dip. Please specify a size in CreateVectorIcon().
    #0 0x000007de6321 in __interceptor_backtrace /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:3980:13
    #1 0x000014b50fcc in base::debug::StackTrace::StackTrace(unsigned long) ./../../base/debug/stack_trace_posix.cc:808:41
    #2 0x000014910423 in logging::LogMessage::~LogMessage() ./../../base/logging.cc:594:29
    #3 0x0000186f43ee in gfx::GetDefaultSizeOfVectorIcon(gfx::VectorIcon const&) ./../../ui/gfx/paint_vector_icon.cc:631:3
    #4 0x000018700c9d in gfx::CreateVectorIcon(gfx::VectorIcon const&, unsigned int) ./../../ui/gfx/paint_vector_icon.cc:603:33
    #5 0x00001f0e75a7 in DeviceChooserContentView::GetIcon(int) ./../../chrome/browser/ui/views/device_chooser_content_view.cc:222:12
    #6 0x00001f0e7a55 in non-virtual thunk to DeviceChooserContentView::GetIcon(int) ./../../chrome/browser/ui/views/device_chooser_content_view.cc:0:0
    #7 0x000017a04f8b in views::TableView::OnPaint(gfx::Canvas*) ./../../ui/views/controls/table/table_view.cc:589:40
    #8 0x000017a911b1 in views::View::Paint(views::PaintInfo const&) ./../../ui/views/view.cc:918:5
    #9 0x000017a9a4bb in views::View::RecursivePaintHelper(void (views::View::*)(views::PaintInfo const&), views::PaintInfo const&) ./../../ui/views/view.cc:2026:7
    #10 0x000017a99d68 in views::View::PaintChildren(views::PaintInfo const&) ./../../ui/views/view.cc:1559:3
    #11 0x000017a9122b in views::View::Paint(views::PaintInfo const&) ./../../ui/views/view.cc:922:3
    #12 0x000017a9a4bb in views::View::RecursivePaintHelper(void (views::View::*)(views::PaintInfo const&), views::PaintInfo const&) ./../../ui/views/view.cc:2026:7
    #13 0x000017a99d68 in views::View::PaintChildren(views::PaintInfo const&) ./../../ui/views/view.cc:1559:3
    #14 0x000017a9122b in views::View::Paint(views::PaintInfo const&) ./../../ui/views/view.cc:922:3
    #15 0x000017a9a4bb in views::View::RecursivePaintHelper(void (views::View::*)(views::PaintInfo const&), views::PaintInfo const&) ./../../ui/views/view.cc:2026:7
    #16 0x000017a99d68 in views::View::PaintChildren(views::PaintInfo const&) ./../../ui/views/view.cc:1559:3
    #17 0x000017a9122b in views::View::Paint(views::PaintInfo const&) ./../../ui/views/view.cc:922:3
    #18 0x000017a9a4bb in views::View::RecursivePaintHelper(void (views::View::*)(views::PaintInfo const&), views::PaintInfo const&) ./../../ui/views/view.cc:2026:7
    #19 0x000017a99d68 in views::View::PaintChildren(views::PaintInfo const&) ./../../ui/views/view.cc:1559:3
    #20 0x000017a9122b in views::View::Paint(views::PaintInfo const&) ./../../ui/views/view.cc:922:3
    #21 0x000017a9a4bb in views::View::RecursivePaintHelper(void (views::View::*)(views::PaintInfo const&), views::PaintInfo const&) ./../../ui/views/view.cc:2026:7
    #22 0x000017a99d68 in views::View::PaintChildren(views::PaintInfo const&) ./../../ui/views/view.cc:1559:3
    #23 0x000017a9122b in views::View::Paint(views::PaintInfo const&) ./../../ui/views/view.cc:922:3
    #24 0x000017a9a4bb in views::View::RecursivePaintHelper(void (views::View::*)(views::PaintInfo const&), views::PaintInfo const&) ./../../ui/views/view.cc:2026:7
    #25 0x000017a99d68 in views::View::PaintChildren(views::PaintInfo const&) ./../../ui/views/view.cc:1559:3
    #26 0x000017a9122b in views::View::Paint(views::PaintInfo const&) ./../../ui/views/view.cc:922:3
    #27 0x000017a9edc8 in views::View::PaintFromPaintRoot(ui::PaintContext const&) ./../../ui/views/view.cc:2033:3
    #28 0x00001ae7af9a in ui::Layer::PaintContentsToDisplayList(cc::ContentLayerClient::PaintingControlSetting) ./../../ui/compositor/layer.cc:1018:16
    #29 0x00001ae7b612 in non-virtual thunk to ui::Layer::PaintContentsToDisplayList(cc::ContentLayerClient::PaintingControlSetting) ./../../ui/compositor/layer.cc:0:0
    #30 0x00001ae83d41 in cc::PictureLayer::Update() ./../../cc/layers/picture_layer.cc:125:39
    #31 0x0000199057fe in cc::LayerTreeHost::PaintContent(std::__1::vector<scoped_refptr<cc::Layer>, std::__1::allocator<scoped_refptr<cc::Layer> > > const&, bool*, bool*) ./../../cc/trees/layer_tree_host.cc:1226:33
    #32 0x000019902d0f in cc::LayerTreeHost::DoUpdateLayers(cc::Layer*) ./../../cc/trees/layer_tree_host.cc:810:28
    #33 0x0000199010b8 in cc::LayerTreeHost::UpdateLayers() ./../../cc/trees/layer_tree_host.cc:669:17
    #34 0x000019bdab0d in DoPainting ./../../cc/trees/single_thread_proxy.cc:778:21
    #35 0x000019bdab0d in cc::SingleThreadProxy::BeginMainFrame(viz::BeginFrameArgs const&) ./../../cc/trees/single_thread_proxy.cc:756:0
    #36 0x000019bddd6d in Invoke<void (cc::SingleThreadProxy::*)(const viz::BeginFrameArgs &), base::WeakPtr<cc::SingleThreadProxy>, viz::BeginFrameArgs> ./../../base/bind_internal.h:447:12
    #37 0x000019bddd6d in MakeItSo<void (cc::SingleThreadProxy::*)(const viz::BeginFrameArgs &), base::WeakPtr<cc::SingleThreadProxy>, viz::BeginFrameArgs> ./../../base/bind_internal.h:567:0
    #38 0x000019bddd6d in RunImpl<void (cc::SingleThreadProxy::*)(const viz::BeginFrameArgs &), std::__1::tuple<base::WeakPtr<cc::SingleThreadProxy>, viz::BeginFrameArgs>, 0, 1> ./../../base/bind_internal.h:621:0
    #39 0x000019bddd6d in base::internal::Invoker<base::internal::BindState<void (cc::SingleThreadProxy::*)(viz::BeginFrameArgs const&), base::WeakPtr<cc::SingleThreadProxy>, viz::BeginFrameArgs>, void ()>::RunOnce(base::internal::BindStateBase*) ./../../base/bind_internal.h:589:0
    #40 0x0000148b0dfe in Run ./../../base/callback.h:96:12
    #41 0x0000148b0dfe in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) ./../../base/debug/task_annotator.cc:101:0
    #42 0x00001493a950 in base::internal::IncomingTaskQueue::RunTask(base::PendingTask*) ./../../base/message_loop/incoming_task_queue.cc:124:19
    #43 0x000014934452 in base::MessageLoop::RunTask(base::PendingTask*) ./../../base/message_loop/message_loop.cc:319:25
    #44 0x000014934fa3 in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) ./../../base/message_loop/message_loop.cc:329:5
    #45 0x0000149358d7 in base::MessageLoop::DoWork() ./../../base/message_loop/message_loop.cc:373:16
    #46 0x000014948107 in HandleDispatch ./../../base/message_loop/message_pump_glib.cc:263:25
    #47 0x000014948107 in base::(anonymous namespace)::WorkSourceDispatch(_GSource*, int (*)(void*), void*) ./../../base/message_loop/message_pump_glib.cc:109:0
    #48 0x7fc3e6401e04 in g_main_context_dispatch ??:0:0
    #49 0x7fc3e6402048 in g_main_context_dispatch ??:?
    #50 0x7fc3e6402048 in ?? ??:0
    #51 0x7fc3e64020ec in g_main_context_iteration ??:0:0
    #52 0x00001494744b in base::MessagePumpGlib::Run(base::MessagePump::Delegate*) ./../../base/message_loop/message_pump_glib.cc:305:30
    #53 0x000014932e72 in base::MessageLoop::Run(bool) ./../../base/message_loop/message_loop.cc:271:12
    #54 0x0000149dcaeb in base::RunLoop::Run() ./../../base/run_loop.cc:130:14
    #55 0x000009db50ed in WaitForDestroy ./../../chrome/browser/ui/test/test_browser_dialog.cc:49:37
    #56 0x000009db50ed in TestBrowserDialog::DismissUi() ./../../chrome/browser/ui/test/test_browser_dialog.cc:155:0
    #57 0x000009db831e in TestBrowserUi::ShowAndVerifyUi() ./../../chrome/browser/ui/test/test_browser_ui.cc:0:5
    #58 0x000016050f52 in content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() ./../../content/public/test/browser_test_base.cc:383:5
    #59 0x000014dd67d7 in Run ./../../base/callback.h:125:12
    #60 0x000014dd67d7 in ChromeBrowserMainParts::PreMainMessageLoopRunImpl() ./../../chrome/browser/chrome_browser_main.cc:2063:0
    #61 0x000014dd371c in ChromeBrowserMainParts::PreMainMessageLoopRun() ./../../chrome/browser/chrome_browser_main.cc:1458:18
    #62 0x00000f13c7f5 in content::BrowserMainLoop::PreMainMessageLoopRun() ./../../content/browser/browser_main_loop.cc:958:13
    #63 0x00001024a1c8 in Run ./../../base/callback.h:125:12
    #64 0x00001024a1c8 in content::StartupTaskRunner::RunAllTasksNow() ./../../content/browser/startup_task_runner.cc:45:0
    #65 0x00000f138775 in content::BrowserMainLoop::CreateStartupTasks() ./../../content/browser/browser_main_loop.cc:871:25
    #66 0x00000f145755 in content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) ./../../content/browser/browser_main_runner.cc:140:17
    #67 0x00000f13109d in content::BrowserMain(content::MainFunctionParams const&) ./../../content/browser/browser_main.cc:42:32
    #68 0x0000144c29f8 in content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) ./../../content/app/content_main_runner.cc:634:14
    #69 0x0000144c6526 in content::ContentMainRunnerImpl::Run() ./../../content/app/content_main_runner.cc:930:12
    #70 0x00001b70b254 in service_manager::Main(service_manager::MainParams const&) ./../../services/service_manager/embedder/main.cc:452:29
    #71 0x0000144bf581 in content::ContentMain(content::ContentMainParams const&) ./../../content/app/content_main.cc:19:10
Forgot to attach the icon: 
Screenshot from 2018-05-01 10-49-17.png
4.0 KB View Download
Labels: -Sheriff-Chromium
removing from sheriff queue given #2.
Status: Started (was: Assigned)
WIP here: https://chromium-review.googlesource.com/c/chromium/src/+/1036684

This changes the bluetooth chooser icon from the first screenshot below to the second, which looks a lot better.
old.png
19.1 KB View Download
new.png
19.1 KB View Download
Cc: est...@chromium.org
+estade@ - calling CreateVectorIcon() with 16x16 instead of baking it into the icon itself results in this screenshot, which doesn't look as nice - what do you think of keeping the original and adding the 16x16 icon as well (seen in new.png of #c5)?
Screenshot from 2018-05-02 10-56-52.png
18.7 KB View Download
hmm. Where did you get the 16x16 definition from if not by (semi-manually) scaling down the 48x48 one?
It was manual :( I downloaded the SVG off of go/icons, then just went into Inkscape to do it.
In that case the outputs ought to be identical and there's a bug here somewhere.
I'm confused what you mean - I went into Inkscape, resized the icon from its original 24x24 (because that is the size I downloaded from go/icons) down to 16x16 and adjusted its artboard size. This changes all the SVG path values to values relative to the new size, so it produces different Skiafy output.

I believe the old.png screenshot above is showing a bad result because it's rasterised onto a 48x48 canvas, then sized down to 16x16, instead of having the canvas draw a SkPath onto the final size?
I think your inkscape is being funny. Look at the start of the icons. For the 48dip svg piped through skiafy we have:

  CANVAS_DIMENSIONS, 48,
  MOVE_TO, 14, 24,
  ...

and if we just scaled this down without trying to be smart, we'd have

  CANVAS_DIMENSIONS, 16,
  MOVE_TO, 4.67, 8,
  ...

PaintVectorIcon doesn't care which of the above it's using to paint; both will look exactly the same onscreen. However, your output from inkscape is

  CANVAS_DIMENSIONS, 16,
  MOVE_TO, 4, 8,
  ... 

because inkscape modified the X value for some unknown reason (probably has some sort of "smart" scaling enabled).
Ah, sorry, by "the outputs should be identical" I thought you meant the .icon output should be identical, but you actually meant the onscreen result should be identical.

Would you prefer not to use the inkscape-formatted icon? Possibly new.png's icon looks better than the one in #c6 because things are then more aligned to the pixel grid. #c6 doesn't look as bad as old.png, so I'll defer to your opinion on this.
yes, I'd prefer if we always use something directly from go/icons or from a designer.
OK, will keep that in mind for the future! 
Project Member

Comment 15 by bugdroid1@chromium.org, May 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3391a0dcca5d37475b5e9604c258c07defcd7562

commit 3391a0dcca5d37475b5e9604c258c07defcd7562
Author: Patti <patricialor@chromium.org>
Date: Mon May 07 02:41:58 2018

Views/Vector Icons: views::TableView now passes the size required for icons.

Refactor TableView and TableModel so that the size of the icon requested is
provided when the TableView is retrieving the icon. This avoids duplicating the
icon size in places other than TableView, and fixes one specific issue where the
bluetooth connected icon used in the device chooser dialog did not specify any
size, triggering a DCHECK.

Bug:  838052 
Change-Id: I14e7463b086c77615286e38100fe7e0f94d8fd34
Reviewed-on: https://chromium-review.googlesource.com/1036684
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556368}
[modify] https://crrev.com/3391a0dcca5d37475b5e9604c258c07defcd7562/chrome/browser/ui/views/device_chooser_content_view.cc
[modify] https://crrev.com/3391a0dcca5d37475b5e9604c258c07defcd7562/ui/base/models/table_model.h
[modify] https://crrev.com/3391a0dcca5d37475b5e9604c258c07defcd7562/ui/views/controls/table/table_view.cc

Status: Fixed (was: Started)

Sign in to add a comment