Fix remaining tests blocking Mojo dispatch changes |
||||||
Issue descriptionIt's time once again to try landing the damn dispatch task granularity change (bug 866708). There are now a few more tests failing, but they are consistent across all platforms. Wew. Gonna try to fix them all in one go. Tests are: content_browsertests: * MediaSessionImplParamBrowserTest.AddingMojoObserverNotifiesCurrentInformation_WithInfo/1 * MediaSessionImplParamBrowserTest.AddingMojoObserverNotifiesCurrentInformation_WithInfo/0 content_unittests: * MediaSessionImplServiceRoutingTest.NotifyMojoObserverMetadataWhenControllable * MediaInternalsAudioFocusTest.AudioFocusStateIsUpdated * MojoAsyncResourceHandlerWithAllocationSizeTest/MojoAsyncResourceHandlerWithAllocationSizeTest.OnWillStartThenOnWillReadThenOnResponseStartedThenOnReadCompletedThenOnResponseCompleted/0 * MojoAsyncResourceHandlerWithAllocationSizeTest/MojoAsyncResourceHandlerWithAllocationSizeTest.OnWillStartThenOnWillReadThenOnResponseStartedThenOnReadCompletedThenOnResponseCompleted/1 * MojoAsyncResourceHandlerWithAllocationSizeTest/MojoAsyncResourceHandlerWithAllocationSizeTest.OnWillStartThenOnResponseStartedThenOnWillReadThenOnReadCompletedThenOnResponseCompleted/0 * MojoAsyncResourceHandlerWithAllocationSizeTest/MojoAsyncResourceHandlerWithAllocationSizeTest.OnWillStartThenOnResponseStartedThenOnWillReadThenOnReadCompletedThenOnResponseCompleted/1 services_unittests: * AudioFocusManagerTest.ObserverActiveSessionChanged/0 * AudioFocusManagerTest.ObserverActiveSessionChanged/1 * MediaControllerTest.ActiveController_Metadata_Observer_WithInfo
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/453a706c8b4fb186a6474fe15fda54346e4ad63b commit 453a706c8b4fb186a6474fe15fda54346e4ad63b Author: Ken Rockot <rockot@google.com> Date: Wed Dec 19 14:31:46 2018 Fix media session and audio focus test behavior When changing Mojo bindings dispatch timing (namely when attempting to land https://chromium-review.googlesource.com/c/chromium/src/+/1145692), a few of these tests break due to incorrect expectations which are no longer being met. Namely: - Empty MediaSessionMetadata updates happen (presumably on sesssion initialization?) and appear to have been previously unanticipated by test code. They happened to be obscured because a single Mojo dispatch task would dispatch the empty metadata message and the populated metadata message all before the test's RunLoop could quit. - The MediaInternalsAudioFocusTest makes some dubious assumptions about the precise number of update callbacks that will be invoked by the system, and this seems to change with the Mojo change in-place. This most likely means that tests waiting on a specific number of callbacks (rather than, say, waiting for specific update contents) at any point is a questionable testing strategy. - AudioFocusManagerTest.ObserverActiveSessionChanged has similar issues, where the system is expected to be in a certain state despite there being insufficient work done to guarantee it. In the first case, I've added a WaitForNonEmptyMetadata() helper to MockMediaSessionMojoObserver which waits more precisely for what the test code wants. This seemed like the best of a few possible options. In the latter two cases, I have cheesily employed RunUntilIdle() to keep the tests enabled without getting stuck trying to sort out the correct expectations, because they seem complicated. Someone should go back and fix the tests later. TBR=xhwang@chromium.org Bug: 916177 Change-Id: I15643e436354f290646f5c66dc3a54b18e99eae0 Reviewed-on: https://chromium-review.googlesource.com/c/1382700 Reviewed-by: Becca Hughes <beccahughes@chromium.org> Commit-Queue: Ken Rockot <rockot@google.com> Cr-Commit-Position: refs/heads/master@{#617823} [modify] https://crrev.com/453a706c8b4fb186a6474fe15fda54346e4ad63b/content/browser/media/media_internals_unittest.cc [modify] https://crrev.com/453a706c8b4fb186a6474fe15fda54346e4ad63b/content/browser/media/session/media_session_impl_browsertest.cc [modify] https://crrev.com/453a706c8b4fb186a6474fe15fda54346e4ad63b/content/browser/media/session/media_session_impl_service_routing_unittest.cc [modify] https://crrev.com/453a706c8b4fb186a6474fe15fda54346e4ad63b/services/media_session/audio_focus_manager_unittest.cc [modify] https://crrev.com/453a706c8b4fb186a6474fe15fda54346e4ad63b/services/media_session/media_controller_unittest.cc [modify] https://crrev.com/453a706c8b4fb186a6474fe15fda54346e4ad63b/services/media_session/public/cpp/test/mock_media_session.cc [modify] https://crrev.com/453a706c8b4fb186a6474fe15fda54346e4ad63b/services/media_session/public/cpp/test/mock_media_session.h
,
Dec 19
Aaaaand now there are more! All Chrome OS, some flaky. These failures are a strong indication that the tests are wrong and relying on unspecified behavior in the system. I have unfortunately been trying to land the Mojo change[1] for several months now, and every time I try there are new test failures that have to be fixed. Probably close to 100 failures have been fixed so far, and so far every single case has been due to incorrect test expectations. +sky@ - any chance you have anyone who can help look into some of these ash/mash failures? Note that they will only repro with [1] applied, but if fixed they should work with or without [1] applied. ash_unittests: HighlighterControllerTest.SelectionInsideScreen single_process_mash_ash_unittests: HighlighterControllerTest.HighlighterGesturesScaled HighlighterControllerTest.SelectionInsideScreen MagnificationControllerTest.TextfieldFocusedWithKeyboard chromeos_unittests: EnableDisableSendingWifiData/SimpleGeolocationWirelessTest.CellularExists/1 EnableDisableSendingWifiData/SimpleGeolocationWirelessTest.WiFiExists/1 simple_process_mash_interactive_ui_tests: ChromeVisibilityObserverInteractiveTest.VisibilityTest LocationIconViewTest.HideOnSecondClick TabDragging/DetachToBrowserTabDragControllerTest.FastResizeDuringDragging/1 TabDragging/DetachToBrowserTabDragControllerTest.DragToSeparateWindow/1 TabDragging/DetachToBrowserTabDragControllerTest.DoNotAttachToOtherWindowTest/1 TabDragging/DetachToBrowserTabDragControllerTest.DragToOverviewNewWindowItem/1 TabDragging/DetachToBrowserTabDragControllerTest.DragToOverviewWindow/1 TabDragging/DetachToBrowserTabDragControllerTest.DragAllToSeparateWindow/1 TabDragging/DetachToBrowserTabDragControllerTest.DragToMinimizedOverviewWindow/1 TabDragging/DetachToBrowserTabDragControllerTest.DragToMinimizedOverviewWindow/0 TabDragging/DetachToBrowserTabDragControllerTest.DragToOverviewNewWindowItem0 TabDragging/DetachToBrowserTabDragControllerTestTouch.SecondFingerPressTest/0 TabDragging/DetachToBrowserTabDragControllerTest.DeferredTargetTabStripTest/1 TabDragging/DetachToBrowserTabDragControllerTest.DragAllToSeparateWindowAndCancel/1 browser_tests: EnterpriseEnrollmentConfigurationTest.TestEnrollUsingToken TopControlsSlideControllerTest.TestDropDowns simple_process_mash_browser_tests: HomeLauncherBrowserNonClientFrameViewAshTest.TabletModeAppCaptionButtonVisibility/1 HomeLauncherBrowserNonClientFrameViewAshTest.TabletModeAppCaptionButtonVisibility/0 BrowserNonClientFrameViewAshTest.HeaderVisibilityInOverviewAndSplitview/1 BrowserNonClientFrameViewAshTest.HeaderVisibilityInOverviewAndSplitview/0 ChromeNativeAppWindowViewsAuraAshBrowserTest.ImmersiveWorkFlow EnterpriseEnrollmentConfigurationTest.TestEnrollUsingToken KeyboardControllerAppWindowTest.DisableOverscrollForImeWindow BrowserNonClientFrameViewAshTest.ToggleTabletModeRelayout/0 BrowserNonClientFrameViewAshTest.ToggleTabletModeRelayout/1 [1] https://chromium-review.googlesource.com/c/chromium/src/+/1145692
,
Dec 19
We are busy trying to fix remaining issues for SingleProcessMash. We will likely have time for this once we get through those issues. Basically, we likely won't have time for this until the new year.
,
Dec 21
Jun or Xiyuan may have some cycles for this soon.
,
Dec 21
Thanks! I have an attempt to "fix" some of them and was considering disabling others: https://chromium-review.googlesource.com/c/chromium/src/+/1388172 Still a few other issues to sort out with that but I think it will be better if the dispatch changes land sooner rather than later.
,
Dec 21
I suspect that the fix for these is to make use of WaitForAllChangesToComplete() in ui/aura/test/mus.
,
Dec 21
I'll take a look at them.
,
Dec 21
(I'm not objecting to disable them for now, but I will see what was missing on those chromeos-mash-specific errors/flakes)
,
Dec 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c44c414dbf96ac6595b1500c2d0f0de8660ed828 commit c44c414dbf96ac6595b1500c2d0f0de8660ed828 Author: Jun Mukai <mukai@chromium.org> Date: Wed Dec 26 18:44:43 2018 Make a wait for Mash on SetAndWaitForTabletMode This fixes the following tests for issue 916177 along with others. TabDragging/DetachToBrowserTabDragControllerTest.DragToMinimizedOverviewWindow/0 TabDragging/DetachToBrowserTabDragControllerTest.DragToMinimizedOverviewWindow/1 Bug: 916177 Test: interactive_ui_tests Change-Id: I9fbc8a81932d1a775bda30cbfb9ac61e901b8b40 Reviewed-on: https://chromium-review.googlesource.com/c/1389439 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Commit-Queue: Jun Mukai <mukai@chromium.org> Cr-Commit-Position: refs/heads/master@{#618944} [modify] https://crrev.com/c44c414dbf96ac6595b1500c2d0f0de8660ed828/chrome/browser/ui/ash/tablet_mode_client_test_util.cc
,
Dec 27
Re: KeyboardControllerAppWindowTest.DisableOverscrollForImeWindow It's very unclear the expectation here; https://chromium-review.googlesource.com/c/chromium/src/+/1388172 changes some order of mojo calls, which causes the actual errors: - without this patch: OnWindowOcclusionChanged happens first, which changes non_ime_app_window's bounds - with this patch: EnsureCaretNotInRect mojo calls happens earlier, which moves non_ime_app_window away so that it won't overlap with the virtual keyboard. Thus the bounds do not change unexpectedly This avoiding of virtual keyboard on appearing does sound reasonable, so the both behavior could be correct. +shend@
,
Dec 27
Hmm unfortunately I'm not the author of the test. From my understanding: - We should first try to move the window away to prevent overlap. - If that's not possible (e.g. window is maximized, or the window size is too big and still overlaps with VK), then we have to also adjust the visual viewport size. If I understand the test correctly, it sets the VK height to be (screen_height - ime_window_visible_height + 1). The "+ 1" means that there should always be an overlap between the app window and the keyboard, even if the window tries to move away. AFAIK, EnsureCaretNotInRect should still update the visual viewport size even after moving the window? e.g. https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_view_aura.cc?sq=package:chromium&g=0&l=1508 +yhanada who probably knows more about this
,
Dec 27
Thanks! I've realized that I was wrong a bit -- I was seeing the window avoidance for showing VK initially. So, the test scenario is 1. show VK, 2. wait for visible/loaded, 3. change the keyboard bounds, and the test scenario doesn't wait for the bounds changed properly when Mash, but previously it happened to succeed since the window was resized for 1. However, waiting explicitly for KeyboardOcclusionBoundsChanged() isn't yet enough, it seems ash_keyboard_ui.cc code doesn't work properly on the bounds changed (or at least doesn't work well with the test scenario). FYI: stevenjb@
,
Dec 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2a915f824bd52357dccf146642048a0fb8bbd3b2 commit 2a915f824bd52357dccf146642048a0fb8bbd3b2 Author: Jun Mukai <mukai@chromium.org> Date: Fri Dec 28 00:24:11 2018 Fix EnterpriseEnrollmentConfigurationTest.TestEnrollUsingToken The test case is broken with https://crrev.com/c/1388172. Turns out that it does not wait for the state changes properly -- calling ExecutePendingJavascript() properly waits for the state changes in the oobe web content. Bug: 916177 Test: browser_tests Change-Id: I58b5730938d882afaebf713a222c01d4c92e2d46 Reviewed-on: https://chromium-review.googlesource.com/c/1391739 Commit-Queue: Jun Mukai <mukai@chromium.org> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Cr-Commit-Position: refs/heads/master@{#619099} [modify] https://crrev.com/2a915f824bd52357dccf146642048a0fb8bbd3b2/chrome/browser/chromeos/login/enterprise_enrollment_browsertest.cc
,
Dec 28
Okay, I think I got all of them, except for KeyboardControllerAppWindowTest.DisableOverscrollForImeWindow. Others are relatively simple and will be addressed by upcoming CLs. KeyboardControllerAppWindowTest is hard for me since it's not clear for me how *actual* VK resize happens. I'll sync up with yhanada@ when he's back.
,
Dec 28
Maybe I can help in the meantime. What do you mean by "actual VK resize"? When the VK shows for the first time, it starts off with 0 height. Then JavaScript will call window.resizeTo and resize the window with the correct VK height. That should cause any windows to move upwards and update the visual viewport sizes.
,
Dec 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3fa5bb24df8a4eb8470d2562e88ee0a347f2d776 commit 3fa5bb24df8a4eb8470d2562e88ee0a347f2d776 Author: Jun Mukai <mukai@chromium.org> Date: Fri Dec 28 17:49:44 2018 Fix HighlighterControllerTest flakiness For crrev.com/c/1388172, some timing of methods calling can change. More specifically, ReclaimResources() can be called after DidLoseLayerTreeFrameSink() is called but it is not deleted. This is revealed as a DCHECK error in ReclaimResources(). See crbug.com/916177 for the details. Bug: 916177 Test: ash_unittests with --gtest_repeat=10 Change-Id: Icf0d16bf94e796c66bd660ad536a7e005bcef042 Reviewed-on: https://chromium-review.googlesource.com/c/1391787 Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org> Commit-Queue: Jun Mukai <mukai@chromium.org> Cr-Commit-Position: refs/heads/master@{#619170} [modify] https://crrev.com/3fa5bb24df8a4eb8470d2562e88ee0a347f2d776/ash/components/fast_ink/fast_ink_view.cc
,
Dec 28
#c16; thanks! As far as I see the vlog, indeed the VK window has 0 height and then gets resized to a height, and that seems properly handled with Mash too. That means I guess JS window.resizeTo works. The test case, DisableOverscrollForImeWindow, does a slightly different thing; after the VK appears (and resized to some default height), the VK window is resized through GetKeyboardWindow()->SetBounds() (in C++). And this SetBounds() seems not properly handled with Mash. Maybe I need to find a better way to mock window.resizeTo which works well with Mash.
,
Jan 2
mukai@ - The SetBounds() timing is tricky with Mash. It requires multiple round trips: JS -> Chrome -> [ash.mojom.KeyboardController.SetOccludedBounds] -> Ash -> [ash.mojom.KeyboardControllerObserver.OnKeyboardOccludedBoundsChanged] -> Chrome -> JS There is a RunUntilIdle() that should probably be a controller->FlushForTesting(): https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/keyboard/keyboard_controller_browsertest.cc?type=cs&q=DisableOverscrollForImeWindow&sq=package:chromium&g=0&l=283 But that might not be sufficient, you might also requrie a JS flush somewhere.
,
Jan 3
stevenjb@ -- I'm afraid that wouldn't work. I actually made a change locally to replace the RunUntilIdle() by waiting OnKeyboardOccludedBoundsChanged(), then realized that the bounds change does not occur occluded bounds change and the test won't finish (uploaded this WIP as https://chromium-review.googlesource.com/c/chromium/src/+/1394595). As analyzed a bit more, I've come to suspect that the keyboard window's resize isn't properly observed in Mash. In classic environment, the window hierarchy is: Root - keyboard container - WebContentsViewAura (= KeyboardWindow) The test code (SetBounds) and window.resizeTo() can update the keyboard window's resize eventually, and so KeyboardOccludedBoundsChange should happen. In Mash: Root - keyboard container - AshKeyboardUI (= KeyboardWindow in Ash) - NativeViewHostAuraClip (in ServerRemoteViewHost) - a native view <--> WebContentsViewAura in the browser (KeyboardWindow in ChromeKeyboardController) window.resizeTo() comes to WebContentsViewAura, which should be propagated to the native view through WS. But I guess that the new bounds isn't propagated to AshKeyboardUI and therefore occluded bounds change never occurs.
,
Jan 3
It is probably because RemoteViewProvider/(Server)RemoteViewHost does not support bounds change from the embedded window. Currently, we make the embedded window change size with its host but not the other way around.
,
Jan 3
IIRC, the code relies on ChromeVirtualKeyboardDelegate::SetOccludedBounds() to notify Ash that the occluded bounds changed, not a window resize. i.e. the extension should be handing the window resize event and calling virtual_keyboard_private.setOclludedBounds(). It's possible that the way that test is implemented is not suited for Mash and needs to be re-implemented. If you want to file a bug and assign it to me, I can take a look (but I appreciate your investigating this, it would be good to have someone else familiar with how the VK works in Mash).
,
Jan 7
Hmm, ChromeVirtualKeyboardDelegate::SetOccludedBounds seems tied with a private virtual keyboard API setOccludedBounds, and I am afraid that this is not the case -- this case resizes the window for VK WebContents, which would end up with occluded bounds change. So I think we need to change ServerRemoteViewHost to support such case as #c21 suggested. Updated my CL.
,
Jan 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/107457cfbd0c28e71ff6e8af40580fbcaae0357c commit 107457cfbd0c28e71ff6e8af40580fbcaae0357c Author: Jun Mukai <mukai@chromium.org> Date: Mon Jan 07 23:46:00 2019 Flush WindowTreeClient before the EventInjector's done callback As with the changes on mojo layer, some tasks can be in window tree client -- and to make the operation order more natural, those tasks need to be finished when the control flow switches to the next task. Note that this shouldn't be WaitForAllChangesToComplete() unlike other changes, since it may not stop at all at this point. Simply flushing is fine. This CL fixes the following tests in issue 916177 LocationIconViewTest.HideOnSecondClick all other TabDragging tests Bug: 916177 Test: interactive_ui_tests Change-Id: I238b3e4f470b64229614a708f337e3548cb130b9 Reviewed-on: https://chromium-review.googlesource.com/c/1389499 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Jun Mukai <mukai@chromium.org> Cr-Commit-Position: refs/heads/master@{#620530} [modify] https://crrev.com/107457cfbd0c28e71ff6e8af40580fbcaae0357c/ui/aura/test/mus/window_tree_client_test_api.h [modify] https://crrev.com/107457cfbd0c28e71ff6e8af40580fbcaae0357c/ui/aura/test/ui_controls_factory_aura.h [modify] https://crrev.com/107457cfbd0c28e71ff6e8af40580fbcaae0357c/ui/aura/test/ui_controls_factory_ozone.cc
,
Jan 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/389ff295112cbdcf60059f0b182ea0a1a96e8f16 commit 389ff295112cbdcf60059f0b182ea0a1a96e8f16 Author: Jun Mukai <mukai@chromium.org> Date: Mon Jan 07 23:46:35 2019 Add WaitForChangesToComplete() in TabDragController tests This fixes many (but not all) of the tab-dragging test failures reported in the issue 916177. Some others are still broken and need to be fixed separately. More specifically, the folloing tests are still failing: TabDragging/DetachToBrowserTabDragControllerTest.DeferredTargetTabStripTest/1 TabDragging/DetachToBrowserTabDragControllerTest.DragToMinimizedOverviewWindow/0 TabDragging/DetachToBrowserTabDragControllerTest.DragToMinimizedOverviewWindow/1 TabDragging/DetachToBrowserTabDragControllerTest.DragToOverviewNewWindowItem/0 TabDragging/DetachToBrowserTabDragControllerTest.DragToOverviewNewWindowItem/1 TabDragging/DetachToBrowserTabDragControllerTest.FastResizeDuringDragging/1 Bug: 916177 Test: interactive_ui_tests with patches Change-Id: I92cdad63dbea6245387d0dceba7120fd7f992cbb Reviewed-on: https://chromium-review.googlesource.com/c/1389322 Commit-Queue: Jun Mukai <mukai@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#620531} [modify] https://crrev.com/389ff295112cbdcf60059f0b182ea0a1a96e8f16/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
,
Jan 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/df9e5f7d297bab04d2dc4ab2d2ee21e1cabc4234 commit df9e5f7d297bab04d2dc4ab2d2ee21e1cabc4234 Author: Jun Mukai <mukai@chromium.org> Date: Mon Jan 07 23:47:06 2019 Use the owned GeolocationHandler instance for tests crrev.com/c/1388172 changes the timing of the callback is called, that leads failures on two SimpleGeolocationWirelessTest test cases. By looking the details, SimpleGeolocationProvider always uses NetworkHandler::Get()->geolocation_handler() while the test case makes sure the status of its own geolocation_handler. This CL allows to use its own geolocaion_handler for SimpleGeolocationProvider. Bug: 916177 Test: chromeos_unittests Change-Id: I56060bdd440591bfa4c2f9bf0b17c114780cee7b Reviewed-on: https://chromium-review.googlesource.com/c/1391869 Reviewed-by: Alexander Alekseev <alemate@chromium.org> Commit-Queue: Jun Mukai <mukai@chromium.org> Cr-Commit-Position: refs/heads/master@{#620532} [modify] https://crrev.com/df9e5f7d297bab04d2dc4ab2d2ee21e1cabc4234/chromeos/geolocation/simple_geolocation_provider.cc [modify] https://crrev.com/df9e5f7d297bab04d2dc4ab2d2ee21e1cabc4234/chromeos/geolocation/simple_geolocation_provider.h [modify] https://crrev.com/df9e5f7d297bab04d2dc4ab2d2ee21e1cabc4234/chromeos/geolocation/simple_geolocation_unittest.cc
,
Jan 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6a1e8aef3b706eff4071b55ce0a851922c369bfa commit 6a1e8aef3b706eff4071b55ce0a851922c369bfa Author: Jun Mukai <mukai@chromium.org> Date: Tue Jan 08 00:57:28 2019 Wait pending changes on closing browser window This fixes ChromeVisibilityObserverInteractiveTest.VisibilityTest Bug: 916177 Test: interactive_ui_tests with the patch Change-Id: Id4a7865c5afd2a5354425d89bfdf779cce925aca Reviewed-on: https://chromium-review.googlesource.com/c/1391352 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Jun Mukai <mukai@chromium.org> Cr-Commit-Position: refs/heads/master@{#620559} [modify] https://crrev.com/6a1e8aef3b706eff4071b55ce0a851922c369bfa/chrome/test/base/in_process_browser_test.cc
,
Jan 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/052fbb5f89174404f604f44929e7f0e963eb6fe0 commit 052fbb5f89174404f604f44929e7f0e963eb6fe0 Author: Jun Mukai <mukai@chromium.org> Date: Tue Jan 08 01:04:54 2019 Fix BrowserNonClientFrameViewAsh tests This CL waits properly for an asynchronous change. Confirmed the following tests are fixed, reported at crbug.com/916177 HomeLauncherBrowserNonClientFrameViewAshTest.TabletModeAppCaptionButtonVisibility/* BrowserNonClientFrameViewAshTest.HeaderVisibilityInOverviewAndSplitview/* Bug: 916177 Test: browser_tests Change-Id: Ic9f98dbb5e4644470ec3b97b0a7119f2ef9aead8 Reviewed-on: https://chromium-review.googlesource.com/c/1391359 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Jun Mukai <mukai@chromium.org> Cr-Commit-Position: refs/heads/master@{#620560} [modify] https://crrev.com/052fbb5f89174404f604f44929e7f0e963eb6fe0/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
,
Jan 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f7cf25475567a68b1f68017efe4269ed2608232d commit f7cf25475567a68b1f68017efe4269ed2608232d Author: Jun Mukai <mukai@chromium.org> Date: Tue Jan 08 01:22:27 2019 Fix TopControlsSlideControllerTest.TestDropDowns This is similar to crrev.com/c/1391739; executing an empty sentence in the web content can wait for the content state updates. Bug: 916177 Test: browser_tests Change-Id: Ia2047a7f7d0179541bfa3c2ed46eea48aca0f5ef Reviewed-on: https://chromium-review.googlesource.com/c/1399518 Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Commit-Queue: Jun Mukai <mukai@chromium.org> Cr-Commit-Position: refs/heads/master@{#620563} [modify] https://crrev.com/f7cf25475567a68b1f68017efe4269ed2608232d/chrome/browser/ui/views/frame/top_controls_slide_controller_chromeos_browsertest.cc
,
Jan 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6a6bcece1c12cce174653ad631b8e0bf2a0058d8 commit 6a6bcece1c12cce174653ad631b8e0bf2a0058d8 Author: Jun Mukai <mukai@chromium.org> Date: Wed Jan 09 00:32:50 2019 wait properly for the keyboard bounds change Instead of RunUntilIdle, it should wait for OnKeyboardOccludedBoundsChange. This CL also allows to resize the window which remotely hosts the VK web-contents based on window.resizeTo(). BUG=916177 TEST=none Change-Id: I5513e8a614c9fef26f92a179adfb25ac86207676 Reviewed-on: https://chromium-review.googlesource.com/c/1394595 Commit-Queue: Jun Mukai <mukai@chromium.org> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Cr-Commit-Position: refs/heads/master@{#620961} [modify] https://crrev.com/6a6bcece1c12cce174653ad631b8e0bf2a0058d8/ash/keyboard/ash_keyboard_ui.cc [modify] https://crrev.com/6a6bcece1c12cce174653ad631b8e0bf2a0058d8/chrome/browser/ui/ash/keyboard/keyboard_controller_browsertest.cc
,
Jan 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/df0b6c576a294ad192adcf2e087e57672ecbf5b7 commit df0b6c576a294ad192adcf2e087e57672ecbf5b7 Author: Hajime Hoshi <hajimehoshi@chromium.org> Date: Wed Jan 09 04:45:56 2019 Revert "wait properly for the keyboard bounds change" This reverts commit 6a6bcece1c12cce174653ad631b8e0bf2a0058d8. Reason for revert: Some test failures on ChromeOS build bots: crbug.com/920073 Original change's description: > wait properly for the keyboard bounds change > > Instead of RunUntilIdle, it should wait for OnKeyboardOccludedBoundsChange. > This CL also allows to resize the window which remotely hosts the VK > web-contents based on window.resizeTo(). > > BUG=916177 > TEST=none > > Change-Id: I5513e8a614c9fef26f92a179adfb25ac86207676 > Reviewed-on: https://chromium-review.googlesource.com/c/1394595 > Commit-Queue: Jun Mukai <mukai@chromium.org> > Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> > Reviewed-by: Steven Bennetts <stevenjb@chromium.org> > Cr-Commit-Position: refs/heads/master@{#620961} TBR=xiyuan@chromium.org,stevenjb@chromium.org,mukai@chromium.org Change-Id: I6220281053f251efa9b9db6e7e384638422033b5 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 916177 Reviewed-on: https://chromium-review.googlesource.com/c/1401926 Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org> Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org> Cr-Commit-Position: refs/heads/master@{#621045} [modify] https://crrev.com/df0b6c576a294ad192adcf2e087e57672ecbf5b7/ash/keyboard/ash_keyboard_ui.cc [modify] https://crrev.com/df0b6c576a294ad192adcf2e087e57672ecbf5b7/chrome/browser/ui/ash/keyboard/keyboard_controller_browsertest.cc
,
Jan 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/59ec9ad1528347e6b2d7146966437bb89c5e9169 commit 59ec9ad1528347e6b2d7146966437bb89c5e9169 Author: Jun Mukai <mukai@chromium.org> Date: Wed Jan 09 22:42:01 2019 Reland "wait properly for the keyboard bounds change" Instead of RunUntilIdle, it should wait for OnKeyboardOccludedBoundsChange. This CL also allows to resize the window which remotely hosts the VK web-contents based on window.resizeTo(). This is a reland of crrev.com/c/1394595 with an addition of waiting the processing of SetBounds in MockEnableIMEInDifferentExtension(). TBR=stevenjb@chromium.org, xiyuan@chromium.org Bug: 916177, 920073 Test: browser_tests Change-Id: Ib353f5193ab4f7bb907a6c02ece6429b42408919 Reviewed-on: https://chromium-review.googlesource.com/c/1403923 Reviewed-by: Jun Mukai <mukai@chromium.org> Commit-Queue: Jun Mukai <mukai@chromium.org> Cr-Commit-Position: refs/heads/master@{#621343} [modify] https://crrev.com/59ec9ad1528347e6b2d7146966437bb89c5e9169/ash/keyboard/ash_keyboard_ui.cc [modify] https://crrev.com/59ec9ad1528347e6b2d7146966437bb89c5e9169/chrome/browser/ui/ash/keyboard/keyboard_controller_browsertest.cc
,
Jan 10
rockot@ -- I believe all of the Mash-related failures/flakiness are now addressed. Please let me know if I'm missing something.
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95800c5106678d024941ebc9e6f4e36a70be1ffe commit 95800c5106678d024941ebc9e6f4e36a70be1ffe Author: Ken Rockot <rockot@google.com> Date: Wed Jan 16 20:57:06 2019 "Fix" Audio focus tests for Mojo dispatch change Changing Mojo dispatch timing breaks these tests: AudioFocusManagerTest.AudioFocusGrouping_TransientResume/2 AudioFocusManagerTest.AudioFocusGrouping_TransientResume/3 This adds some extra synchronization to the tests so that they end up in the expected state. Bug: 916177 Change-Id: If0915a50e43f66240e4b09a5ac0517de219a8bbc Reviewed-on: https://chromium-review.googlesource.com/c/1409813 Commit-Queue: Ken Rockot <rockot@google.com> Reviewed-by: Becca Hughes <beccahughes@chromium.org> Cr-Commit-Position: refs/heads/master@{#623350} [modify] https://crrev.com/95800c5106678d024941ebc9e6f4e36a70be1ffe/services/media_session/audio_focus_manager_unittest.cc |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bugdroid1@chromium.org
, Dec 19