New issue
Advanced search Search tips

Issue 837438 link

Starred by 5 users

Issue metadata

Status: Assigned
Owner:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Connect ScopedFakeNSWindowFullscreen to WindowedNotificationObserver NOTIFICATION_FULLSCREEN_CHANGED

Project Member Reported by robliao@chromium.org, Apr 26 2018

Issue description

ScopedFakeNSWindowFullscreen just makes the window bigger without going through the full motions of getting there. This breaks tests waiting for chrome::NOTIFICATION_FULLSCREEN_CHANGED.

Cocoa does it through this stack:
(lldb) bt 20
* thread #1, name = 'CrBrowserMain', queue = 'com.apple.main-thread', stop reason = breakpoint 2.1
  * frame #0: 0x000000010925f2e3 interactive_ui_tests`FullscreenController::PostFullscreenChangeNotification(this=0x0000000158f345f8, is_fullscreen=true) at fullscreen_controller.cc:306
    frame #1: 0x000000010925f7a9 interactive_ui_tests`FullscreenController::WindowFullscreenStateChanged(this=0x0000000158f345f8) at fullscreen_controller.cc:259
    frame #2: 0x00000001091e44f8 interactive_ui_tests`Browser::WindowFullscreenStateChanged(this=0x0000000158f12470) at browser.cc:867
    frame #3: 0x00000001096edc62 interactive_ui_tests`::-[BrowserWindowController(self=0x0000000158f25e80, _cmd="windowDidEnterFullScreen:", notification="NSWindowDidEnterFullScreenNotification") windowDidEnterFullScreen:](NSNotification *) at browser_window_controller_private.mm:657
    frame #4: 0x00007fff4444753c CoreFoundation`__invoking___ + 140
    frame #5: 0x00007fff44447410 CoreFoundation`-[NSInvocation invoke] + 320
    frame #6: 0x00000001098777a3 interactive_ui_tests`::-[TabWindowControllerProxy forwardInvocation:](self=0x000000015dd205a0, _cmd="forwardInvocation:", anInvocation=0x000000016db02950) at tab_window_controller.mm:72
    frame #7: 0x00007fff44445ebc CoreFoundation`___forwarding___ + 732
    frame #8: 0x00007fff44445b58 CoreFoundation`__forwarding_prep_0___ + 120
    frame #9: 0x00007fff4445f61c CoreFoundation`__CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + 12
    frame #10: 0x00007fff4445f4ea CoreFoundation`_CFXRegistrationPost + 458
    frame #11: 0x00007fff4445f221 CoreFoundation`___CFXNotificationPost_block_invoke + 225
    frame #12: 0x00007fff4441dd72 CoreFoundation`-[_CFXNotificationRegistrar find:object:observer:enumerator:] + 1826
    frame #13: 0x00007fff4441ce03 CoreFoundation`_CFXNotificationPost + 659
    frame #14: 0x00007fff465398c7 Foundation`-[NSNotificationCenter postNotificationName:object:userInfo:] + 66
    frame #15: 0x0000000105233ef9 interactive_ui_tests`ui::test::ScopedFakeNSWindowFullscreen::Impl::FinishEnterFullscreen(this=0x000000015d8b39c0, fullscreen_content_size=(width = 1920, height = 1200)) at scoped_fake_nswindow_fullscreen.mm:134

 
Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 27 2018

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

commit a78a27ebf69d7a4709a41dd80d1c80baca6208be
Author: Robert Liao <robliao@chromium.org>
Date: Fri Apr 27 16:48:13 2018

Skip Waiting for Fullscreen for Mac in BrowserCommandControllerInteractiveTest.ShortcutsShouldTakeEffectInWindowMode

ScopedFakeNSWindowFullscreen just makes the NSWindow larger without
taking Chrome into fullscreen. As a result,
chrome::NOTIFICATION_FULLSCREEN_CHANGED does not fire.

Fullscreen appears to be performed synchronously with the keyboard
event, so skipping the wait works.

BUG=834754,837438

Change-Id: I337a44eefedfd382cc253aaf69590140fc557ba1
Reviewed-on: https://chromium-review.googlesource.com/1031617
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554407}
[modify] https://crrev.com/a78a27ebf69d7a4709a41dd80d1c80baca6208be/chrome/browser/ui/browser_command_controller_interactive_browsertest.cc

Labels: Group-Tests
Labels: Target-70 M-70
Owner: ellyjo...@chromium.org
Gonna need this for issue 823478.

Sign in to add a comment