[SingleProcessMash] omnibox leaves results window |
||||||
Issue descriptionToT(commit position: 619702) Hitting return/escape in omnibox doesn't close the results window.
,
Jan 7
,
Jan 7
This may be related to key event processing that is causing another bugs. See bug 919183 .
,
Jan 7
,
Jan 7
We had this before as issue 902547 (first half). The problem back then is because we somehow lost proper local surface id for omnibox popup widget and compositor stops rendering. As a result, the close animation never finishes and the popup up widget stays. It was fixed by msw's https://chromium-review.googlesource.com/c/chromium/src/+/1321867. I wonder whether we got similar issue again.
,
Jan 8
Oshima saw this on a slate, which runs with a scale factor of 2.25. AFAICT this issue is specific to fractional scale factors. I can repro on the desktop if I run with a scale factor of 1.25 (or 2.25, as the slate does). When the omnibox popup is shown the opacity of the Layer animates from 0 to 1. When the bug happens the opacity animation seems to stop part way through (often times at the first tick), so that the cc::Layer (and ui::Layer) think the opacity is 0. By animation I specifically mean ThreadedOpacityTransition. In addition to the animation stopping BeginFrames is no longer called. Here's the trace for when OnNeedBeginFrames is called with false: 0x7f99f25a556d base::debug::StackTrace::StackTrace() #1 0x7f99f22a089a base::debug::StackTrace::StackTrace() #2 0x7f99bc0228df cc::mojo_embedder::AsyncLayerTreeFrameSink::OnNeedsBeginFrames() #3 0x7f99e8508f73 viz::ExternalBeginFrameSource::RemoveObserver() #4 0x7f99e6877311 cc::Scheduler::StartOrStopBeginFrames() #5 0x7f99e687546f cc::Scheduler::ProcessScheduledActions() #6 0x7f99e687a09d cc::Scheduler::FinishImplFrame() #7 0x7f99e687aa6e cc::Scheduler::OnBeginImplFrameDeadline() #8 0x7f99e687d6ad _ZN4base8internal13FunctorTraitsIMN2cc9SchedulerEFvvEvE6InvokeIS5_PS3_JEEEvT_OT0_DpOT1_ #9 0x7f99e687d5f4 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIMN2cc9SchedulerEFvvEJPS5_EEEvOT_DpOT0_ #10 0x7f99e687d5a5 _ZN4base8internal7InvokerINS0_9BindStateIMN2cc9SchedulerEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE7RunImplIS6_NSt4__Cr5tupleIJS8_EEEJLm0EEEEvOT\ _OT0_NSD_16integer_sequenceImJXspT1_EEEE #11 0x7f99e687d4e9 _ZN4base8internal7InvokerINS0_9BindStateIMN2cc9SchedulerEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE7RunOnceEPNS0_13BindStateBaseE #12 0x7f99e687d9fe _ZNO4base12OnceCallbackIFvvEE3RunEv #13 0x7f99e687d934 _ZN4base8internal22CancelableCallbackImplINS_12OnceCallbackIFvvEEEE11ForwardOnceIJEEEvDpT_ #14 0x7f99e6780d6f _ZN4base8internal13FunctorTraitsIMN2cc28ScrollbarAnimationControllerEFvvEvE6InvokeIS5_RKNS_7WeakPtrIS3_EEJEEEvT_OT0_DpOT1_ #15 0x7f99e687db6a _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIMNS0_22CancelableCallbackImplINS_12OnceCallbackIFvvEEEEEFvvENS_7WeakPtrIS8_EEJEEEvOT_OT0_DpOT1_ #16 0x7f99e687db00 _ZN4base8internal7InvokerINS0_9BindStateIMNS0_22CancelableCallbackImplINS_12OnceCallbackIFvvEEEEEFvvEJNS_7WeakPtrIS7_EEEEES5_E7RunImplIS9_NSt4__\ Cr5tupleIJSB_EEEJLm0EEEEvOT_OT0_NSF_16integer_sequenceImJXspT1_EEEE #17 0x7f99e687da49 _ZN4base8internal7InvokerINS0_9BindStateIMNS0_22CancelableCallbackImplINS_12OnceCallbackIFvvEEEEEFvvEJNS_7WeakPtrIS7_EEEEES5_E7RunOnceEPNS0_13Bi\ ndStateBaseE Closing the popup results in another animation that animates opacity to 0. As ui::Layer and cc::Layer think the opacity is 0, the animation does nothing. The Widget/WindowTreeHost/Window backing the popup are destroyed when the animation completes. Because the animation never starts, it never completes, and the widget is left on screen. I'm not sure why, but this seems to be the result of WindowTreeHostMus bouncing around in size. The size bouncing is no doubt because of differences in coordinate conversion.
,
Jan 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e910f9b0be3a4c791f021a3cc33f481d6d420261 commit e910f9b0be3a4c791f021a3cc33f481d6d420261 Author: Scott Violet <sky@chromium.org> Date: Tue Jan 08 22:23:36 2019 views: make coordinate conversion consistent Widget::SetBounds() converts DIPs to pixels using Screen::DIPToScreenRectInWindow(). Screen::DIPToScreenRectInWindow() uses enclosing rect. In window-service related code we scale the size separately. This is done to avoid the size being dependant on the location. The fix is to add a SetBoundsInDIP to DesktopWindowTreeHost so that DesktopWindowTreeHostMus can have a different implementation. BUG= 919250 TEST=covered by test Change-Id: I0d38e7af8631fe3a4ea5cc2c785c2ed9adad41e0 Reviewed-on: https://chromium-review.googlesource.com/c/1401236 Commit-Queue: Scott Violet <sky@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#620906} [modify] https://crrev.com/e910f9b0be3a4c791f021a3cc33f481d6d420261/ui/views/BUILD.gn [modify] https://crrev.com/e910f9b0be3a4c791f021a3cc33f481d6d420261/ui/views/mus/desktop_window_tree_host_mus.cc [modify] https://crrev.com/e910f9b0be3a4c791f021a3cc33f481d6d420261/ui/views/mus/desktop_window_tree_host_mus.h [modify] https://crrev.com/e910f9b0be3a4c791f021a3cc33f481d6d420261/ui/views/mus/desktop_window_tree_host_mus_unittest.cc [modify] https://crrev.com/e910f9b0be3a4c791f021a3cc33f481d6d420261/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc [add] https://crrev.com/e910f9b0be3a4c791f021a3cc33f481d6d420261/ui/views/widget/desktop_aura/desktop_window_tree_host.cc [modify] https://crrev.com/e910f9b0be3a4c791f021a3cc33f481d6d420261/ui/views/widget/desktop_aura/desktop_window_tree_host.h [modify] https://crrev.com/e910f9b0be3a4c791f021a3cc33f481d6d420261/ui/views/widget/desktop_aura/desktop_window_tree_host_platform.cc [modify] https://crrev.com/e910f9b0be3a4c791f021a3cc33f481d6d420261/ui/views/widget/desktop_aura/desktop_window_tree_host_platform.h
,
Jan 8
,
Jan 9
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by jamescook@google.com
, Jan 7