New issue
Advanced search Search tips

Issue 650849 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Oct 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 547071



Sign in to add a comment

Prepare chrome/browser/fullscreen_mac.mm for 10.9 deployment target.

Project Member Reported by erikc...@chromium.org, Sep 27 2016

Issue description

FAILED: obj/chrome/browser/browser/fullscreen_mac.o 
/Users/erikchen/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/chrome/browser/browser/fullscreen_mac.o.d -DV8_DEPRECATION_WARNINGS -DENABLE_NOTIFICATIONS -DENABLE_PEPPER_CDMS -DENABLE_PLUGINS=1 -DENABLE_PDF=1 -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1 -DENABLE_PRINT_PREVIEW=1 -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1 -DNO_TCMALLOC -DUSE_EXTERNAL_POPUP_MENU=1 -DENABLE_WEBRTC=1 -DENABLE_EXTENSIONS=1 -DENABLE_TASK_MANAGER=1 -DENABLE_THEMES=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_SUPERVISED_USERS=1 -DENABLE_SERVICE_DISCOVERY=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=282097-1 -DCR_XCODE_VERSION=0720 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORE=0 -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DUSE_CUPS -DTOOLKIT_VIEWS=1 -DSK_IGNORE_DW_GRAY_FIX -DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS -DSK_SUPPORT_GPU=1 -DSK_BUILD_FOR_MAC -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_NOEXCEPT= -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DHAVE_PTHREAD -DENABLE_IPC_FUZZER -DV8_USE_EXTERNAL_STARTUP_DATA -DENABLE_WEBSOCKETS -DMESA_EGL_NO_X11_HEADERS -DLEVELDB_PLATFORM_CHROMIUM=1 -DI18N_ADDRESSINPUT_USE_BASICTYPES_OVERRIDE=1 -DI18N_ADDRESS_VALIDATION_DATA_URL=\"https://chromium-i18n.appspot.com/ssl-aggregate-address/\" -DFEATURE_ENABLE_SSL -DFEATURE_ENABLE_VOICEMAIL -DEXPAT_RELATIVE_PATH -DGTEST_RELATIVE_PATH -DNO_MAIN_THREAD_WRAPPING -DNO_SOUND_SYSTEM -DWEBRTC_CHROMIUM_BUILD -DWEBRTC_POSIX -DWEBRTC_MAC -DXML_STATIC -DSSL_USE_OPENSSL -DHAVE_OPENSSL_SSL_H -DFEATURE_ENABLE_SSL -DLOGGING=1 -DNO_MAIN_THREAD_WRAPPING -DHUNSPELL_STATIC -DHUNSPELL_CHROME_CLIENT -DUSE_HUNSPELL -I../.. -Igen -I../../third_party/khronos -I../../gpu -I../../skia/config -I../../skia/ext -I../../third_party/skia/include/c -I../../third_party/skia/include/config -I../../third_party/skia/include/core -I../../third_party/skia/include/effects -I../../third_party/skia/include/images -I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops -I../../third_party/skia/include/pdf -I../../third_party/skia/include/pipe -I../../third_party/skia/include/ports -I../../third_party/skia/include/utils -I../../third_party/skia/include/gpu -I../../third_party/skia/src/gpu -I../../third_party/skia/src/sksl -I../../third_party/ced/src -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -Igen/chrome -I../../third_party/protobuf/src -Igen/protoc_out -I../../third_party/protobuf/src -Igen/components/strings -Igen/components/strings -Igen/components/strings -Igen/components/strings -I../../third_party/WebKit -Igen/third_party/WebKit -I../../v8/include -Igen/v8/include -Igen -I../../v8/include -Igen/v8/include -I../../third_party/libwebm/source -I../../third_party/opus/src/include -I../../third_party/boringssl/src/include -I../../third_party/re2/src -I../../third_party/mesa/src/include -Igen/ui/resources -Igen/ui/resources -Igen -Igen/extensions -Igen/extensions -Igen/extensions -Igen -Igen/extensions/strings -I../../third_party/google_toolbox_for_mac -I../../third_party/google_toolbox_for_mac/src -I../../third_party/google_toolbox_for_mac/src/AppKit -I../../third_party/google_toolbox_for_mac/src/DebugUtils -I../../third_party/google_toolbox_for_mac/src/Foundation -Igen/chrome -Igen/chrome -Igen/chrome -Igen/ui/views/resources -I../../third_party/cacheinvalidation/overrides -I../../third_party/cacheinvalidation/src -I../../third_party/leveldatabase -I../../third_party/leveldatabase/src -I../../third_party/leveldatabase/src/include -Igen/components -Igen/components -I../../third_party/libaddressinput/src/cpp/include -I../../third_party/libaddressinput/chromium/override -I../../third_party/webrtc_overrides -I../../testing/gtest/include -I../../third_party -I../../third_party/webrtc_overrides -I../../third_party -I../../third_party/expat/files/lib -I../../third_party/libxml/src/include -I../../third_party/libxml/mac/include -I../../third_party/libyuv -I../../third_party/libyuv/include -I../../third_party/zlib -Igen -fno-strict-aliasing -fstack-protector-strong -fcolor-diagnostics -arch x86_64 -Wall -Werror -Wextra -Wpartial-availability -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-deprecated-register -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-shift-negative-value -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -O0 -g1 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk -mmacosx-version-min=10.9 -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.dylib -Xclang -add-plugin -Xclang find-bad-constructs -Wheader-hygiene -Wstring-conversion -Wexit-time-destructors -Wno-unused-function -DLIBXML_STATIC= -fno-threadsafe-statics -fvisibility-inlines-hidden -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare -std=c++11 -stdlib=libc++ -fobjc-call-cxx-cdtors -Wobjc-missing-property-synthesis -fno-rtti -fno-exceptions -c ../../chrome/browser/fullscreen_mac.mm -o obj/chrome/browser/browser/fullscreen_mac.o
../../chrome/browser/fullscreen_mac.mm:13:7: error: 'CGDisplayIsCaptured' is deprecated: first deprecated in macOS 10.9 [-Werror,-Wdeprecated-declarations]
  if (CGDisplayIsCaptured(CGMainDisplayID()))
      ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/CoreGraphics.framework/Headers/CGDirectDisplay.h:299:21: note: 'CGDisplayIsCaptured' has been explicitly marked deprecated here
CG_EXTERN boolean_t CGDisplayIsCaptured(CGDirectDisplayID display)
                    ^
1 error generated.

 

Comment 1 by tapted@chromium.org, Sep 29 2016

Cc: a...@chromium.org jianli@chromium.org
Code is

bool IsFullScreenMode() {
  // Check if the main display has been captured (by games in particular).
  if (CGDisplayIsCaptured(CGMainDisplayID()))
    return true;

Added in 72539 -> https://codereview.chromium.org/6359008 "Do not show notifications when in fullscreen or screensaver mode."

In r178470 -> https://chromiumcodereview.appspot.com/12052012 "Switch to Cocoa APIs for fullscreen." ( Issue 170189 ) avi@ added more checks, using [NSApp currentSystemPresentationOptions].

I think checking whether the menu bar is unavailable is a good signal for when to show notifications. (i.e. "NSApplicationPresentationHideMenuBar-> "Menu Bar is entirely unavailable" according to NSApplication.h).

So, IMO we can just delete the CGDisplayIsCaptured check.


Regarding CGDisplayIsCaptured, https://developer.apple.com/reference/coregraphics/1562061-cgdisplayiscaptured

says "Returns a Boolean value indicating whether a display is captured. Deprecated. There is no replacement."

But there is:

/* Return a CGContext suitable for drawing to the captured display
   `display', or NULL if `display' has not been captured. The context is
   owned by the device and should not be released by the caller.

   The context remains valid while the display is captured and while the
   display configuration is unchanged. Releasing the captured display or
   reconfiguring the display invalidates the drawing context.

   The determine when the display configuration is changing, use
   `CGDisplayRegisterReconfigurationCallback'. */

CG_EXTERN CGContextRef CGDisplayGetDrawingContext(CGDirectDisplayID display)
  CG_AVAILABLE_STARTING(__MAC_10_3, __IPHONE_NA);


But I don't think it's worth trying to use it. it's unclear what it means to use that method if another application has captured the display.

Comment 2 by tapted@chromium.org, Oct 10 2016

Status: Started (was: Untriaged)
https://codereview.chromium.org/2406753003

yah - tested with a fullscreen game from 2011 and the screensaver. CGDisplayIsCaptured() does seem redundant here.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 10 2016

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

commit 5cde3b4543c7d79f23e9b15dd70318cba70e57ff
Author: tapted <tapted@chromium.org>
Date: Mon Oct 10 22:56:14 2016

Mac: Remove deprecated call to CGDisplayIsCaptured IsFullScreenMode().

The call was added in r72539 to ensure notifications are not shown when
another app has fullscreen (e.g. games or the screensaver).

r178470 added additional calls using Cocoa APIs which made it redundant,
so just delete the call to CGDisplayIsCaptured().

Verified that attempts to show notifications when running an old
fullscreen game or with the screensaver active are correctly suppressed.
Traced to ensure CGDisplayIsCaptured() would have been responsible for
doing the same in these cases.

BUG= 650849 

Review-Url: https://codereview.chromium.org/2406753003
Cr-Commit-Position: refs/heads/master@{#424272}

[modify] https://crrev.com/5cde3b4543c7d79f23e9b15dd70318cba70e57ff/chrome/browser/fullscreen_mac.mm

Comment 4 by tapted@chromium.org, Oct 10 2016

Status: Fixed (was: Started)

Sign in to add a comment