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

Issue 580973003: Revert of Mac: Fix accidental changes to fullscreen logic from refactor. (Closed)

Created:
6 years, 3 months ago by Dan Beam
Modified:
6 years, 3 months ago
Reviewers:
Robert Sesek, erikchen
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@fullscreen_layout_refactor3
Project:
chromium
Visibility:
Public.

Description

Revert of Mac: Fix accidental changes to fullscreen logic from refactor. (patchset #2 id:40001 of https://codereview.chromium.org/575653003/) Reason for revert: Seems to break tests on Mac ASAN 64 Tests (3): https://build.chromium.org/p/chromium.memory/builders/Mac%20ASan%2064%20Tests%20%283%29/builds/2665 AppShimInteractiveTest.Launch (run #1): [ RUN ] AppShimInteractiveTest.Launch [3265:26883:0917/140040:ERROR:launch_services_util.cc(58)] LSOpenApplication: procNotFound (-600) BrowserTestBase signal handler received SIGTERM. Backtrace: 0 interactive_ui_tests 0x000000010b22c753 base::debug::StackTrace::StackTrace() + 19 1 interactive_ui_tests 0x0000000115c3ce69 content::(anonymous namespace)::DumpStackTraceSignalHandler(int) + 185 2 libsystem_c.dylib 0x00007fff8688f90a _sigtramp + 26 3 ??? 0x000060e000009dc0 0x0 + 106515188981184 4 CoreFoundation 0x00007fff86b16233 __CFRunLoopServiceMachPort + 195 5 CoreFoundation 0x00007fff86b1b916 __CFRunLoopRun + 1078 6 CoreFoundation 0x00007fff86b1b0e2 CFRunLoopRunSpecific + 290 7 HIToolbox 0x00007fff8f4c8eb4 RunCurrentEventLoopInMode + 209 8 HIToolbox 0x00007fff8f4c8c52 ReceiveNextEventCommon + 356 9 HIToolbox 0x00007fff8f4c8ae3 BlockUntilNextEventMatchingListInMode + 62 10 AppKit 0x00007fff8cc36533 _DPSNextEvent + 685 11 AppKit 0x00007fff8cc35df2 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 128 12 AppKit 0x00007fff8cc2d1a3 -[NSApplication run] + 517 13 interactive_ui_tests 0x000000010b1f5186 base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) + 886 14 interactive_ui_tests 0x000000010b1f3b5a base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) + 474 15 interactive_ui_tests 0x000000010b2fbae2 base::RunLoop::Run() + 530 16 interactive_ui_tests 0x00000001088df0f8 apps::AppShimInteractiveTest_Launch_Test::RunTestOnMainThread() + 12296 17 interactive_ui_tests 0x0000000109e08b7a InProcessBrowserTest::RunTestOnMainThreadLoop() + 810 18 interactive_ui_tests 0x0000000115c3c633 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() + 995 19 interactive_ui_tests 0x0000000108ea85d4 ChromeBrowserMainParts::PreMainMessageLoopRunImpl() + 12132 20 interactive_ui_tests 0x0000000108ea5288 ChromeBrowserMainParts::PreMainMessageLoopRun() + 296 21 interactive_ui_tests 0x0000000112d726a7 content::BrowserMainLoop::PreMainMessageLoopRun() + 375 22 interactive_ui_tests 0x000000011349f906 content::StartupTaskRunner::RunAllTasksNow() + 294 23 interactive_ui_tests 0x0000000112d6df2d content::BrowserMainLoop::CreateStartupTasks() + 1901 24 interactive_ui_tests 0x0000000112d79558 content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) + 1000 25 interactive_ui_tests 0x0000000112d67754 content::BrowserMain(content::MainFunctionParams const&) + 404 26 interactive_ui_tests 0x0000000115c39b0c content::ContentMainRunnerImpl::Run() + 476 27 interactive_ui_tests 0x0000000115c37c2e content::ContentMain(content::ContentMainParams const&) + 142 28 interactive_ui_tests 0x0000000115c3ba37 content::BrowserTestBase::SetUp() + 1799 29 interactive_ui_tests 0x0000000109e065e9 InProcessBrowserTest::SetUp() + 745 30 interactive_ui_tests 0x000000010a510647 testing::Test::Run() + 327 31 interactive_ui_tests 0x000000010a512ac1 testing::TestInfo::Run() + 2049 32 interactive_ui_tests 0x000000010a513772 testing::TestCase::Run() + 1042 33 interactive_ui_tests 0x000000010a5285dd testing::internal::UnitTestImpl::RunAllTests() + 4173 34 interactive_ui_tests 0x000000010a527505 testing::UnitTest::Run() + 389 35 interactive_ui_tests 0x0000000115c35115 base::TestSuite::Run() + 565 36 interactive_ui_tests 0x0000000108db6494 InteractiveUITestSuiteRunner::RunTestSuite(int, char**) + 212 37 interactive_ui_tests 0x0000000115c46cc3 content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) + 1763 38 interactive_ui_tests 0x0000000109e03f14 LaunchChromeTests(int, ChromeTestSuiteRunner*, int, char**) + 244 39 interactive_ui_tests 0x0000000108db6352 main + 194 40 interactive_ui_tests 0x00000001088dbb34 start + 52 41 ??? 0x000000000000000a 0x0 + 10 Original issue's description: > Mac: Fix accidental changes to fullscreen logic from refactor. > > The menu item "Enter Presentation Mode" has different effects in 10.6 and > 10.7+. In 10.6, the menu item invokes Immersive Fullscreen. In 10.7+, the menu > item invokes AppKit Fullscreen. Similar logic applies to fullscreen invoked by > an extension. > > I accidentally changed this logic during the fullscreen refactor by removing > the 10.6 vs 10.7+ conditional. This CL adds back the conditional. I also > updated the code to explicitly indicate the mechanism that is invoking > fullscreen to prevent similar future mistakes. > > BUG=NONE > > Committed: https://crrev.com/507054ccac1778eace34f76afd40ade6e855d6fc > Cr-Commit-Position: refs/heads/master@{#295321} TBR=rsesek@chromium.org,erikchen@chromium.org NOTREECHECKS=true NOTRY=true BUG=NONE Committed: https://crrev.com/055cf1e9b20f5de4909d5199616c9c80e02ba9d9 Cr-Commit-Position: refs/heads/master@{#295368}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -68 lines) Patch
M chrome/browser/ui/cocoa/browser_window_cocoa.mm View 2 chunks +3 lines, -15 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_controller.h View 2 chunks +5 lines, -33 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_controller.mm View 2 chunks +7 lines, -20 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Dan Beam
Created Revert of Mac: Fix accidental changes to fullscreen logic from refactor.
6 years, 3 months ago (2014-09-17 23:00:09 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/580973003/1
6 years, 3 months ago (2014-09-17 23:00:59 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1) as 82fb25f382d5acf79cd910efb9562b6165e9a70a
6 years, 3 months ago (2014-09-17 23:01:56 UTC) #3
commit-bot: I haz the power
6 years, 3 months ago (2014-09-17 23:02:31 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/055cf1e9b20f5de4909d5199616c9c80e02ba9d9
Cr-Commit-Position: refs/heads/master@{#295368}

Powered by Google App Engine
This is Rietveld 408576698