Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(132)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1829583004: Fix for an issue with the Fullscreen Menu Bar on OSX (Closed)

Created:
4 years, 9 months ago by spqchan
Modified:
4 years, 9 months ago
Reviewers:
Robert Sesek, dgrogan
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix for an issue with the Fullscreen Menu Bar on OSX Modified the MenuBarRevealHandler so that it also handles kEventMenuBarShown and kEventBarHidden events. Made sure that the handler would ignore carbon events of the menu bars in other Spaces. BUG=414115 Committed: https://crrev.com/fbfe6adf70739a45dd34bb609f2c4f56b07e19ec Cr-Commit-Position: refs/heads/master@{#383081}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+48 lines, -18 lines) Patch
M chrome/browser/ui/cocoa/presentation_mode_controller.h View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/presentation_mode_controller.mm View 5 chunks +45 lines, -18 lines 0 comments Download

Messages

Total messages: 14 (6 generated)
spqchan
PTAL
4 years, 9 months ago (2016-03-23 21:06:25 UTC) #3
Robert Sesek
LGTM
4 years, 9 months ago (2016-03-24 16:24:18 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1829583004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1829583004/1
4 years, 9 months ago (2016-03-24 16:30:30 UTC) #6
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 9 months ago (2016-03-24 17:06:46 UTC) #8
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/fbfe6adf70739a45dd34bb609f2c4f56b07e19ec Cr-Commit-Position: refs/heads/master@{#383081}
4 years, 9 months ago (2016-03-24 17:07:52 UTC) #10
dgrogan
BrowserWindowControllerTest.FullscreenToolbarIsVisibleAccordingToPrefs broke recently on Mac 10.11. Could you look into it to see if it ...
4 years, 9 months ago (2016-03-25 00:29:18 UTC) #12
spqchan
On 2016/03/25 00:29:18, dgrogan wrote: > BrowserWindowControllerTest.FullscreenToolbarIsVisibleAccordingToPrefs broke > recently on Mac 10.11. Could you ...
4 years, 9 months ago (2016-03-25 01:35:20 UTC) #13
spqchan
4 years, 9 months ago (2016-03-25 02:29:11 UTC) #14
Message was sent while issue was closed. 
On 2016/03/25 01:35:20, spqchan wrote:
> On 2016/03/25 00:29:18, dgrogan wrote:
> > BrowserWindowControllerTest.FullscreenToolbarIsVisibleAccordingToPrefs broke
> > recently on Mac 10.11. Could you look into it to see if it was this change?
> > 
> > The test first timed out then failed on retry.
> > 
> >
>
https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac10.11%20Tests...
> > 
> > [ RUN      ]
> > BrowserWindowControllerTest.FullscreenToolbarIsVisibleAccordingToPrefs
> > 2016-03-24 13:01:49.246 browser_tests[38661:370496] NSWindow warning: adding
> an
> > unknown subview: <FullSizeContentView: 0x7fb7cb2666b0>. Break on NSLog to
> debug.
> > 2016-03-24 13:01:49.247 browser_tests[38661:370496] Call stack:
> > (
> >     "+callStackSymbols disabled for performance reasons"
> > )
> > [38664:1295:0324/130149:WARNING:ipc_message_attachment_set.cc(57)]
> > MessageAttachmentSet destroyed with unconsumed descriptors: 0/1
> > [38665:1295:0324/130149:ERROR:HTMLTreeBuilder.cpp(2525)] Not implemented
> reached
> > in void blink::HTMLTreeBuilder::defaultForInitial()
> > [38665:1295:0324/130149:ERROR:HTMLTreeBuilder.cpp(2466)] Not implemented
> reached
> > in void blink::HTMLTreeBuilder::processEndOfFile(blink::AtomicHTMLToken *)
> > [38665:1295:0324/130149:ERROR:HTMLTreeBuilder.cpp(2525)] Not implemented
> reached
> > in void blink::HTMLTreeBuilder::defaultForInitial()
> > [38665:1295:0324/130149:ERROR:HTMLTreeBuilder.cpp(2466)] Not implemented
> reached
> > in void blink::HTMLTreeBuilder::processEndOfFile(blink::AtomicHTMLToken *)
> > BrowserTestBase signal handler received SIGTERM. Backtrace:
> > 0   browser_tests                       0x000000010510e203
> > _ZN4base5debug10StackTraceC1Ev + 19
> > 1   browser_tests                       0x00000001051c22f1
> > _ZN7content12_GLOBAL__N_1L27DumpStackTraceSignalHandlerEi + 65
> > 2   libsystem_platform.dylib            0x00007fff89fad52a _sigtramp + 26
> > 3   browser_tests                       0x000000010516d7d8
> > _ZN4base9TimeTicks3NowEv + 40
> > 4   CoreFoundation                      0x00007fff868b82b4
> > __CFRunLoopServiceMachPort + 212
> > 5   CoreFoundation                      0x00007fff868b777c __CFRunLoopRun +
> 1356
> > 6   CoreFoundation                      0x00007fff868b6fc8
> CFRunLoopRunSpecific
> > + 296
> > 7   HIToolbox                           0x00007fff8b683d55
> > RunCurrentEventLoopInMode + 235
> > 8   HIToolbox                           0x00007fff8b683b8f
> > ReceiveNextEventCommon + 432
> > 9   HIToolbox                           0x00007fff8b6839cf
> > _BlockUntilNextEventMatchingListInModeWithFilter + 71
> > 10  AppKit                              0x00007fff8f999d96 _DPSNextEvent +
> 1067
> > 11  AppKit                              0x00007fff8f9991c5 -[NSApplication
> > _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 454
> > 12  AppKit                              0x00007fff8f98dd28 -[NSApplication
> run]
> > + 682
> > 13  browser_tests                       0x0000000105103b36
> > _ZN4base24MessagePumpNSApplication5DoRunEPNS_11MessagePump8DelegateE + 326
> > 14  browser_tests                       0x0000000105103154
> > _ZN4base24MessagePumpCFRunLoopBase3RunEPNS_11MessagePump8DelegateE + 100
> > 15  browser_tests                       0x00000001051474a3
> > _ZN4base7RunLoop3RunEv + 99
> > 16  browser_tests                       0x0000000109a45acf
> > _ZN7content14RunThisRunLoopEPN4base7RunLoopE + 79
> > 17  browser_tests                       0x0000000109a467ae
> > _ZN7content28WindowedNotificationObserver4WaitEv + 126
> > 18  browser_tests                       0x0000000104b9a09a
> > _ZN27BrowserWindowControllerTest38ToggleFullscreenAndWaitForNotificationEv +
> 90
> > 19  browser_tests                       0x0000000104b9a608
> >
>
_ZN75BrowserWindowControllerTest_FullscreenToolbarIsVisibleAccordingToPrefs_Test19RunTestOnMainThreadEv
> > + 392
> > 20  browser_tests                       0x00000001050fc439
> > _ZN20InProcessBrowserTest23RunTestOnMainThreadLoopEv + 169
> > 21  browser_tests                       0x00000001051c20db
> > _ZN7content15BrowserTestBase28ProxyRunTestOnMainThreadLoopEv + 235
> > 22  browser_tests                       0x0000000104c85036
> > _ZN22ChromeBrowserMainParts25PreMainMessageLoopRunImplEv + 3958
> > 23  browser_tests                       0x0000000104c83f8a
> > _ZN22ChromeBrowserMainParts21PreMainMessageLoopRunEv + 122
> > 24  browser_tests                       0x00000001085bed03
> > _ZN7content15BrowserMainLoop21PreMainMessageLoopRunEv + 179
> > 25  browser_tests                       0x00000001088791b0
> > _ZN7content17StartupTaskRunner14RunAllTasksNowEv + 48
> > 26  browser_tests                       0x00000001085bd2e4
> > _ZN7content15BrowserMainLoop18CreateStartupTasksEv + 644
> > 27  browser_tests                       0x00000001085c122a
> > _ZN7content21BrowserMainRunnerImpl10InitializeERKNS_18MainFunctionParamsE +
> 538
> > 28  browser_tests                       0x00000001085bab55
> > _ZN7content11BrowserMainERKNS_18MainFunctionParamsE + 149
> > 29  browser_tests                       0x0000000109a14394
> > _ZN7content21ContentMainRunnerImpl3RunEv + 100
> > 30  browser_tests                       0x0000000109a13866
> > _ZN7content11ContentMainERKNS_17ContentMainParamsE + 54
> > 31  browser_tests                       0x00000001051c1e6c
> > _ZN7content15BrowserTestBase5SetUpEv + 940
> > 32  browser_tests                       0x00000001050fb8f7
> > _ZN20InProcessBrowserTest5SetUpEv + 231
> > 33  browser_tests                       0x00000001055b3a8b
> > _ZN7testing4Test3RunEv + 299
> > 34  browser_tests                       0x00000001055b46ca
> > _ZN7testing8TestInfo3RunEv + 410
> > 35  browser_tests                       0x00000001055b4c93
> > _ZN7testing8TestCase3RunEv + 451
> > 36  browser_tests                       0x00000001055bb229
> > _ZN7testing8internal12UnitTestImpl11RunAllTestsEv + 825
> > 37  browser_tests                       0x00000001055baebd
> > _ZN7testing8UnitTest3RunEv + 269
> > 38  browser_tests                       0x00000001051a5c59
> > _ZN4base9TestSuite3RunEv + 217
> > 39  browser_tests                       0x00000001050e303f
> > _ZN21ChromeTestSuiteRunner12RunTestSuiteEiPPc + 31
> > 40  browser_tests                       0x0000000109a42edf
> > _ZN7content11LaunchTestsEPNS_20TestLauncherDelegateEiiPPc + 319
> > 41  browser_tests                       0x0000000104ac898a main + 90
> > 42  browser_tests                       0x0000000103fd6f34 start + 52
> > [3868/5028]
> > BrowserWindowControllerTest.FullscreenToolbarIsVisibleAccordingToPrefs
(TIMED
> > OUT)
> > 
> > [ RUN      ]
> > BrowserWindowControllerTest.FullscreenToolbarIsVisibleAccordingToPrefs
> > 2016-03-24 13:21:43.385 browser_tests[47182:474738] NSWindow warning: adding
> an
> > unknown subview: <FullSizeContentView: 0x7feb41eb5680>. Break on NSLog to
> debug.
> > 2016-03-24 13:21:43.385 browser_tests[47182:474738] Call stack:
> > (
> >     "+callStackSymbols disabled for performance reasons"
> > )
> > [47185:1295:0324/132143:WARNING:ipc_message_attachment_set.cc(57)]
> > MessageAttachmentSet destroyed with unconsumed descriptors: 0/1
> > [47186:1295:0324/132143:ERROR:HTMLTreeBuilder.cpp(2525)] Not implemented
> reached
> > in void blink::HTMLTreeBuilder::defaultForInitial()
> > [47186:1295:0324/132143:ERROR:HTMLTreeBuilder.cpp(2466)] Not implemented
> reached
> > in void blink::HTMLTreeBuilder::processEndOfFile(blink::AtomicHTMLToken *)
> > [47186:1295:0324/132143:ERROR:HTMLTreeBuilder.cpp(2525)] Not implemented
> reached
> > in void blink::HTMLTreeBuilder::defaultForInitial()
> > [47186:1295:0324/132143:ERROR:HTMLTreeBuilder.cpp(2466)] Not implemented
> reached
> > in void blink::HTMLTreeBuilder::processEndOfFile(blink::AtomicHTMLToken *)
> > ../../chrome/browser/ui/cocoa/browser_window_controller_browsertest.mm:330:
> > Failure
> > Expected: (NSMinY(toolbarFrame)) >= (NSMaxY(screenFrame)), actual: 746 vs
768
> > [  FAILED  ]
> > BrowserWindowControllerTest.FullscreenToolbarIsVisibleAccordingToPrefs,
where
> > TypeParam =  and GetParam() =  (2333 ms)
> 
> It most likely is, since the menubar can get flaky. I'll see what I can do to
> fix it.
> Feel free to revert if necessary

I ran the test several times and it looks like the test is very flaky. Most of
the time, my change passed with the test, but occasionally it fails.

Powered by Google App Engine
This is Rietveld 408576698