New issue
Advanced search Search tips

Issue 650845 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 547071



Sign in to add a comment

Prepare chrome/browser/ui/cocoa/browser_window_utils.mm for 10.9 deployment target.

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

Issue description

FAILED: obj/chrome/browser/ui/ui/browser_window_utils.o 
/Users/erikchen/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/chrome/browser/ui/ui/browser_window_utils.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 -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DHAVE_PTHREAD -DENABLE_IPC_FUZZER -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 -DV8_USE_EXTERNAL_STARTUP_DATA -DENABLE_WEBSOCKETS -DMESA_EGL_NO_X11_HEADERS -DLEVELDB_PLATFORM_CHROMIUM=1 -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 -DI18N_ADDRESSINPUT_USE_BASICTYPES_OVERRIDE=1 -DI18N_ADDRESS_VALIDATION_DATA_URL=\"https://chromium-i18n.appspot.com/ssl-aggregate-address/\" -Igen/chrome/browser/ui -I../.. -Igen -I../../third_party/khronos -I../../gpu -I../../third_party/protobuf/src -Igen/protoc_out -I../../third_party/protobuf/src -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 -Igen -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -Igen/ui/views/resources -Igen/chrome -Igen/chrome -Igen/chrome -Igen/chrome -I../../third_party/ced/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 -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 -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/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/zlib -I../../third_party/libaddressinput/src/cpp/include -I../../third_party/libaddressinput/chromium/override -Igen/third_party/libaddressinput -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 -Wno-nonnull -Wexit-time-destructors -Wno-unused-function -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/ui/cocoa/browser_window_utils.mm -o obj/chrome/browser/ui/ui/browser_window_utils.o
../../chrome/browser/ui/cocoa/browser_window_utils.mm:153:3: error: 'GetCurrentProcess' is deprecated: first deprecated in macOS 10.9 [-Werror,-Wdeprecated-declarations]
  GetCurrentProcess(&psn);
  ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/ApplicationServices.framework/Frameworks/HIServices.framework/Headers/Processes.h:415:1: note: 'GetCurrentProcess' has been explicitly marked deprecated here
MacGetCurrentProcess(ProcessSerialNumber * PSN)               AVAILABLE_MAC_OS_X_VERSION_10_0_AND_LATER_BUT_DEPRECATED_IN_MAC_OS_X_VERSION_10_9;
^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/ApplicationServices.framework/Frameworks/HIServices.framework/Headers/Processes.h:412:34: note: expanded from macro 'MacGetCurrentProcess'
    #define MacGetCurrentProcess GetCurrentProcess
                                 ^
../../chrome/browser/ui/cocoa/browser_window_utils.mm:154:3: error: 'SetFrontProcessWithOptions' is deprecated: first deprecated in macOS 10.9 [-Werror,-Wdeprecated-declarations]
  SetFrontProcessWithOptions(&psn, kSetFrontProcessFrontWindowOnly);
  ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/ApplicationServices.framework/Frameworks/HIServices.framework/Headers/Processes.h:658:1: note: 'SetFrontProcessWithOptions' has been explicitly marked deprecated here
SetFrontProcessWithOptions(
^
2 errors generated.

 
Components: UI>Browser
Labels: OS-Mac
Owner: rsesek@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 4 2016

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

commit c57771b074f8c3b43592908ddcb513535064e0c8
Author: rsesek <rsesek@chromium.org>
Date: Tue Oct 04 22:41:40 2016

[Mac] Remove usage of deprecated Carbon SetFrontProcessWithOptions().

Replace with -[NSRunningApplication activateWithOptions:].

BUG= 650845 
R=avi@chromium.org

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

[modify] https://crrev.com/c57771b074f8c3b43592908ddcb513535064e0c8/chrome/browser/ui/cocoa/browser_window_utils.mm

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 6 2016

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

commit 6d3dbe8eb6ff81ed2c9e89df2e13d1764056e9d7
Author: rsesek <rsesek@chromium.org>
Date: Thu Oct 06 18:14:41 2016

Use -[NSApp activateIgnoringOtherApps:NO] instead of -[NSRunningApplication activateWithOptions:].

In c57771b074f8, +[BrowserWindowUtils activateWindowController:] was changed
from using Carbon's SetFrontProcessWithOptions(). This caused an activation bug,
 https://crbug.com/653483 , to occur. The difference in behavior is because the
NSApplication method internally calls _NXActivateSelf(), which uses
SetFrontProcessWithOptions(). The NSRunningApplication method instead messages
LaunchServices to set the front process.

The NSRunningApplication method appers to deactivate the app if it is currently
active, which caused the bug. Switching to the NSApplication method resolves
the issue and restores the old behavior. An alternatieve to using the
NSApplication method would be to make calling the NSRunningApplication method
conditional on -[NSApp isActive].

BUG= 653483 , 650845 
R=avi@chromium.org

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

[modify] https://crrev.com/6d3dbe8eb6ff81ed2c9e89df2e13d1764056e9d7/chrome/browser/ui/cocoa/browser_window_utils.mm

Project Member

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

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

commit d694190ff0e1e1d594ccd644f807a1309ee61d5e
Author: sdy <sdy@chromium.org>
Date: Mon Oct 10 18:02:34 2016

[Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome.

Passing NO doesn't activate our app in most cases; it seems to be mainly
useful when an app is launched (so there may be a long delay before it's
ready to become active) but the user might have switched to another app
in the mean time. In that case, the activation isn't the immediate
result of a user gesture, so it shouldn't steal focus.

This is a recent regression from crrev.com/2386343004.

BUG= 654441 , 653483 , 650845 

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

[modify] https://crrev.com/d694190ff0e1e1d594ccd644f807a1309ee61d5e/chrome/browser/ui/cocoa/browser_window_utils.mm

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d3dbe8eb6ff81ed2c9e89df2e13d1764056e9d7

commit 6d3dbe8eb6ff81ed2c9e89df2e13d1764056e9d7
Author: rsesek <rsesek@chromium.org>
Date: Thu Oct 06 18:14:41 2016

Use -[NSApp activateIgnoringOtherApps:NO] instead of -[NSRunningApplication activateWithOptions:].

In c57771b074f8, +[BrowserWindowUtils activateWindowController:] was changed
from using Carbon's SetFrontProcessWithOptions(). This caused an activation bug,
 https://crbug.com/653483 , to occur. The difference in behavior is because the
NSApplication method internally calls _NXActivateSelf(), which uses
SetFrontProcessWithOptions(). The NSRunningApplication method instead messages
LaunchServices to set the front process.

The NSRunningApplication method appers to deactivate the app if it is currently
active, which caused the bug. Switching to the NSApplication method resolves
the issue and restores the old behavior. An alternatieve to using the
NSApplication method would be to make calling the NSRunningApplication method
conditional on -[NSApp isActive].

BUG= 653483 , 650845 
R=avi@chromium.org

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

[modify] https://crrev.com/6d3dbe8eb6ff81ed2c9e89df2e13d1764056e9d7/chrome/browser/ui/cocoa/browser_window_utils.mm

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 27 2016

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

commit dcb9b2152f0875df5fa0352b92da1ee97e2bba89
Author: Robert Sesek <rsesek@chromium.org>
Date: Wed Oct 12 23:16:57 2016

[Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome.

Passing NO doesn't activate our app in most cases; it seems to be mainly
useful when an app is launched (so there may be a long delay before it's
ready to become active) but the user might have switched to another app
in the mean time. In that case, the activation isn't the immediate
result of a user gesture, so it shouldn't steal focus.

This is a recent regression from crrev.com/2386343004.

BUG= 654441 , 653483 , 650845 

Review-Url: https://codereview.chromium.org/2409623002
Cr-Commit-Position: refs/heads/master@{#424182}
(cherry picked from commit d694190ff0e1e1d594ccd644f807a1309ee61d5e)

Review URL: https://codereview.chromium.org/2415783002 .

Cr-Commit-Position: refs/branch-heads/2883@{#77}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/dcb9b2152f0875df5fa0352b92da1ee97e2bba89/chrome/browser/ui/cocoa/browser_window_utils.mm

Comment 8 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment