New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 735328 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 732652
issue 726828
issue 734737
issue 736309



Sign in to add a comment

Need clang roll

Project Member Reported by infe...@chromium.org, Jun 21 2017

Issue description

Need clang roll >= 305871 to uptake https://reviews.llvm.org/rL305871
 

Comment 1 by thakis@chromium.org, Jun 21 2017

Likely not soon, we just rolled yesterday.

Your stuff seems to break a lot, please think about automated testing to make that happen less frequently. At least 50% of roll fires are due to issues on CF.
Cc: euge...@chromium.org vitalyb...@chromium.org
Labels: -Pri-1 Pri-2
This can go in the next clang roll. I just wanted to file a tracking bug.

Evgeniy, Vitaly, any thoughts on how this can be tested. I think the tests make sense on LLVM side for this and not on chromium side.
Correct. We even had the test for that, which failed if executed manually.
Unfortunately we have no bots with Android. We need to setup something.

Comment 4 by r...@chromium.org, Jun 23 2017

Blocking: 736309
We need clang r306137 to fix  crbug.com/736309 .

Comment 5 by r...@chromium.org, Jun 23 2017

Cc: r...@chromium.org

Comment 6 by p...@chromium.org, Jul 5 2017

Blocking: 732652

Comment 7 by p...@chromium.org, Jul 5 2017

Cc: p...@chromium.org

Comment 8 by p...@chromium.org, Jul 6 2017

Going to try for r307286. ToT bots for Windows and Linux look good, but Mac/iOS are failing with -Wunguarded-availability warnings, e.g.

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.fyi%2FClangToTMac%2F14496%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.fyi%2FClangToTiOS%2F11463%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

Seems to have been caused by r307175.

I'll fix these with local pragmas disabling the warning if there aren't too many of them, otherwise I'll disable it globally.

Comment 9 by p...@chromium.org, Jul 6 2017

Owner: p...@chromium.org
Hm disabling that globally wouldn't be good, we added that warning so that we don't accidentally call new functions on old OSs. If we disable it, we're flying blind in that regard. It's cool that Erik made the warning much better, but we need some transition story (and I haven't found any docs for the new way and I'm unsure how exactly it works). I asked some questions on that commit.
Cc: sdy@chromium.org
+sdy for comment 10. See also cfe-commits for r307175

Comment 12 by p...@chromium.org, Jul 6 2017

Makes sense, I'll see if I can feasibly disable it locally then. (5 disablements and counting after having built about half of everything so far for Mac and iOS.)

Comment 13 by p...@chromium.org, Jul 7 2017

It turned out to be not so hard to convert the code over to using @available and the availability attribute, which is what
https://chromium-review.googlesource.com/562980 does. It builds fine locally, but needs some cleanup and probably needs to be split into multiple CLs. I'll look at that tomorrow.

For that to work we'll need to ship the builtins runtime library in our clang package, which is what https://chromium-review.googlesource.com/c/563157/ does. I'll land that first and see what the trybots think of my other CL.
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 7 2017

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

commit 0b54ae5cc48704bc3ab1ff0c5fa6d5155c3c9eef
Author: Peter Collingbourne <pcc@chromium.org>
Date: Fri Jul 07 20:36:52 2017

Add builtin archives to the clang package for macOS.

These archives contain the definition of _IsOSVersionAtLeast, which
we will need in order to use the @available function.

BUG= 735328 
R=thakis@chromium.org

Change-Id: Ifa3b419493da2361cbd7df4ba993c0d7fe9d5cbd
Reviewed-on: https://chromium-review.googlesource.com/563157
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485031}
[modify] https://crrev.com/0b54ae5cc48704bc3ab1ff0c5fa6d5155c3c9eef/tools/clang/scripts/package.py
[modify] https://crrev.com/0b54ae5cc48704bc3ab1ff0c5fa6d5155c3c9eef/tools/clang/scripts/update.py

Comment 15 by r...@chromium.org, Jul 7 2017

Blocking: 726828

Comment 16 by r...@chromium.org, Jul 7 2017

My fix for  crbug.com/736309  was bad and I reverted it. It's the main reason why the Windows ToT testers are red. I'll try to get to that today.

Comment 17 by p...@chromium.org, Jul 7 2017

I don't follow. Is that the fix that you made in r306137 and reverted in r306175? If so I don't see why it would only be affecting the ToT bots, we rolled before then.

Anyway, it looks like there's a separate problem at compile time that was introduced somewhere between r307413 and r307436. Nothing in that range looks immediately suspicious.

https://build.chromium.org/p/chromium.fyi/builders/ClangToTWin/builds/12868
https://build.chromium.org/p/chromium.fyi/builders/ClangToTWin/builds/12869

Comment 18 by r...@chromium.org, Jul 7 2017

> I don't follow. Is that the fix that you made in r306137 and reverted in r306175? If so I don't see why it would only be affecting the ToT bots, we rolled before then.

It doesn't only affect ToT bots, it's just the reason that a lot of them are red right now. My attempts to workaround the bug in Chromium didn't fix the pinned release+dll bots, which you can see here:
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang64%28dll%29%20tester/builds/6111 (content_unittests failures)

Anyway, I just landed a fix for this dllimport member pointer thing in r307446. If we can roll past that, we should fix  crbug.com/736309 .

I'll look into the new compile failure, I guess.

Comment 19 by p...@chromium.org, Jul 7 2017

I see, thanks.
Project Member

Comment 20 by bugdroid1@chromium.org, Jul 10 2017

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

commit def049dfc8ce2054b7a64106ed6d1d2999e7df64
Author: Peter Collingbourne <pcc@chromium.org>
Date: Mon Jul 10 18:08:07 2017

Protect NSVisualEffectView references with availability or @available.

An upcoming version of clang removes the ability to suppress
availability warnings by redeclaring functions. The new way
to suppress warnings is to either annotate the caller with an
availability attribute or enclose the function reference in an "if
(@available)" block.

Because the type NSVisualEffectView is marked as only available
in 10.10 onwards, so must the visualEffectView property in
TabWindowController and all other references to NSVisualEffectView.
Any code that uses functions on NSVisualEffectView must be protected
with @available.

Bug:  735328 
Change-Id: Ibe1bb965ae93bd0becc24fb90356462b1a26ab86
Reviewed-on: https://chromium-review.googlesource.com/564289
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485312}
[modify] https://crrev.com/def049dfc8ce2054b7a64106ed6d1d2999e7df64/chrome/browser/ui/cocoa/tabs/tab_strip_view.mm
[modify] https://crrev.com/def049dfc8ce2054b7a64106ed6d1d2999e7df64/chrome/browser/ui/cocoa/tabs/tab_window_controller.h
[modify] https://crrev.com/def049dfc8ce2054b7a64106ed6d1d2999e7df64/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm

Project Member

Comment 21 by bugdroid1@chromium.org, Jul 10 2017

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

commit 9c85ac23dfdacf01298e1d7698632d62649a99a0
Author: Peter Collingbourne <pcc@chromium.org>
Date: Mon Jul 10 18:17:39 2017

Use @available to protect references to NS45Notification.

An upcoming version of clang removes the ability to suppress
availability warnings by redeclaring functions. The new way
to suppress warnings is to either annotate the caller with an
availability attribute or enclose the function reference in an "if
(@available)" block.

This patch uses @available to protect references to
NSWorkspaceAccessibilityDisplayOptionsDidChangeNotification,
instead of comparing its address to zero.

Bug:  735328 
Change-Id: If6f428da62c1486c1d608f0afe5637f9fb2f4e07
Reviewed-on: https://chromium-review.googlesource.com/564151
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485315}
[modify] https://crrev.com/9c85ac23dfdacf01298e1d7698632d62649a99a0/chrome/browser/ui/cocoa/tabs/tab_view.mm

Project Member

Comment 22 by bugdroid1@chromium.org, Jul 10 2017

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

commit 7040dce0051600239d1a9a549022e29c51c77fc0
Author: Peter Collingbourne <pcc@chromium.org>
Date: Mon Jul 10 18:23:07 2017

Rename function in ToolbarController to avoid availability warning.

An upcoming version of clang removes the ability to suppress
availability warnings by redeclaring functions. This means that any
calls to "viewDidLoad" in this code will trigger an availability
warning, because that function was added to the API in 10.10.

To suppress the warning, rename the function to viewDidLoadImpl and
introduce a wrapper function viewDidLoad that calls viewDidLoadImpl.

Bug:  735328 
Change-Id: Ieb806578cc860b483fc75023fb39a7a4af171e14
Reviewed-on: https://chromium-review.googlesource.com/564209
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485317}
[modify] https://crrev.com/7040dce0051600239d1a9a549022e29c51c77fc0/chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm

Project Member

Comment 23 by bugdroid1@chromium.org, Jul 10 2017

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

commit 3f975c269a8ddc69b806ece592757b019b2c086a
Author: Peter Collingbourne <pcc@chromium.org>
Date: Mon Jul 10 18:27:17 2017

Rename function in BookmarkBarController to avoid availability warning.

An upcoming version of clang removes the ability to suppress
availability warnings by redeclaring functions. This means that any
calls to "viewDidLoad" in this code will trigger an availability
warning, because that function was added to the API in 10.10.

To suppress the warning, rename the function to viewDidLoadImpl and
introduce a wrapper function viewDidLoad that calls viewDidLoadImpl.

Bug:  735328 
Change-Id: Ifcea5787d095601dd621636380770a2c8d7acb59
Reviewed-on: https://chromium-review.googlesource.com/564095
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485322}
[modify] https://crrev.com/3f975c269a8ddc69b806ece592757b019b2c086a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
[modify] https://crrev.com/3f975c269a8ddc69b806ece592757b019b2c086a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_view_cocoa_unittest.mm

Project Member

Comment 24 by bugdroid1@chromium.org, Jul 10 2017

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

commit f4d9f75e5079ef2b48870554dcb7cbd44ee4efd6
Author: Peter Collingbourne <pcc@chromium.org>
Date: Mon Jul 10 19:42:44 2017

Use @available to control call to UUIDString.

An upcoming version of clang removes the ability to suppress
availability warnings by redeclaring functions. The new way
to suppress warnings is to either annotate the caller with an
availability attribute or enclose the function reference in an "if
(@available)" block.

This patch does the latter for a call to CBUUID::UUIDString. The
function was asserting that the OS version was at least 10.10, so
preserve the existing behaviour by asserting false on the else branch.

Bug:  735328 
Change-Id: I4db8e20ebf6db7eef1ef1f63997ec2a778bc65ee
Reviewed-on: https://chromium-review.googlesource.com/565027
Reviewed-by: Vincent Scheib <scheib@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485351}
[modify] https://crrev.com/f4d9f75e5079ef2b48870554dcb7cbd44ee4efd6/device/bluetooth/bluetooth_adapter_mac.mm

Project Member

Comment 25 by bugdroid1@chromium.org, Jul 10 2017

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

commit cf43fdad3f08929e2c12e9f397e818ddfd7340dc
Author: Peter Collingbourne <pcc@chromium.org>
Date: Mon Jul 10 20:20:02 2017

Use @available to compute style mask.

An upcoming version of clang removes the ability to suppress
availability warnings by redeclaring functions. The new way
to suppress warnings is to either annotate the caller with an
availability attribute or enclose the function reference in an "if
(@available)" block.

The constant NSFullSizeContentViewWindowMask is only available in
10.10 onwards, so only mask it in from that version onwards.

Bug:  735328 
Change-Id: I89c08e31ef887dffc6634a6e7def52b243232cf1
Reviewed-on: https://chromium-review.googlesource.com/564342
Reviewed-by: ccameron chromium <ccameron@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485362}
[modify] https://crrev.com/cf43fdad3f08929e2c12e9f397e818ddfd7340dc/chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm

Project Member

Comment 26 by bugdroid1@chromium.org, Jul 10 2017

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

commit fc74af31e49db0d577943059053b9e02d08f51ee
Author: Peter Collingbourne <pcc@chromium.org>
Date: Mon Jul 10 21:00:18 2017

Use @available and availability in AppController and HandoffManager.

An upcoming version of clang removes the ability to suppress
availability warnings by redeclaring functions. The new way
to suppress warnings is to either annotate the caller with an
availability attribute or enclose the function reference in an "if
(@available)" block.

The functions willContinueUserActivityWithType and continueUserActivity
are called only by the operating system, so we can use availability
attributes. The function passURLToHandoffManager, however, may
be called or overridden from the test suite even before 10.10 (see
SetUpInProcessBrowserTestFixture in app_controller_mac_browsertest.mm),
so we need to use a runtime check.

Any references to NSUserActivity must have availability attributes
because that type was introduced in 10.10. The test-only parts
of HandoffManager are only used in an iOS-specific test, so for
simplicity we disable that code under non-iOS.

Bug:  735328 
Change-Id: Ide1dd1cdec9f8a2c286ddd09d6e4ca6da2117e6a
Reviewed-on: https://chromium-review.googlesource.com/564226
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485379}
[modify] https://crrev.com/fc74af31e49db0d577943059053b9e02d08f51ee/chrome/browser/app_controller_mac.mm
[modify] https://crrev.com/fc74af31e49db0d577943059053b9e02d08f51ee/components/handoff/handoff_manager.h
[modify] https://crrev.com/fc74af31e49db0d577943059053b9e02d08f51ee/components/handoff/handoff_manager.mm

Project Member

Comment 27 by bugdroid1@chromium.org, Jul 10 2017

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

commit a0cb3f441ee6c89bd231e4a96923ec3b93980dd2
Author: Peter Collingbourne <pcc@chromium.org>
Date: Mon Jul 10 21:19:09 2017

Use @available in service_process_util_mac.mm.

An upcoming version of clang removes the ability to suppress
availability warnings by redeclaring functions. The new way
to suppress warnings is to either annotate the caller with an
availability attribute or enclose the function reference in an "if
(@available)" block.

This patch changes service_process_util_mac.mm to use the
SDK's definition of the getRelationship function instead of a
redeclaration and protect the call with an @available check instead
of respondsToSelector.

Bug:  735328 
Change-Id: I2097b2343f63b74cbc198e7cf7bcaabea2e45735
Reviewed-on: https://chromium-review.googlesource.com/564308
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485395}
[modify] https://crrev.com/a0cb3f441ee6c89bd231e4a96923ec3b93980dd2/chrome/common/service_process_util_mac.mm

Project Member

Comment 28 by bugdroid1@chromium.org, Jul 10 2017

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

commit a02c0cd8c869ab4bd798add97f6dda85ce0a6763
Author: Peter Collingbourne <pcc@chromium.org>
Date: Mon Jul 10 22:53:15 2017

Change crdmg's minimum SDK version to 10.10.

An upcoming version of clang removes the ability to suppress
availability warnings by redeclaring functions. The new way
to suppress warnings is to either annotate the caller with an
availability attribute or enclose the function reference in an "if
(@available)" block.

But we don't need to do any of that for crdmg. According to source
code comments, the executable only works with 10.10+, so we can adjust
the minimum SDK version to match.

Bug:  735328 
Change-Id: Ib735ffe60c67681b2d13f5dbbfddb6aa6b928248
Reviewed-on: https://chromium-review.googlesource.com/564290
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Reviewed-by: Sidney San Martin <sdy@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485433}
[modify] https://crrev.com/a02c0cd8c869ab4bd798add97f6dda85ce0a6763/chrome/utility/safe_browsing/mac/BUILD.gn
[modify] https://crrev.com/a02c0cd8c869ab4bd798add97f6dda85ce0a6763/chrome/utility/safe_browsing/mac/crdmg.cc

Comment 29 by p...@chromium.org, Jul 11 2017

The compile failure that I mentioned in c#17 was reported as https://bugs.llvm.org/show_bug.cgi?id=33721 and appears to have been fixed as of r307486. I'll prepare a roll to that revision so that we're ready to go once the remaining availability fixes have landed:

https://chromium-review.googlesource.com/c/565028/
https://chromium-review.googlesource.com/c/564224/
https://chromium-review.googlesource.com/c/564225/
Project Member

Comment 30 by bugdroid1@chromium.org, Jul 11 2017

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

commit 58299c8150a3f7248a986eec7d87210a6abf0896
Author: Peter Collingbourne <pcc@chromium.org>
Date: Tue Jul 11 01:20:27 2017

Use @available to protect a reference to CIDetectorTypeQRCode.

An upcoming version of clang removes the ability to suppress
availability warnings by redeclaring functions. The new way
to suppress warnings is to either annotate the caller with an
availability attribute or enclose the function reference in an "if
(@available)" block.

This patch does the latter for a reference to
CIDetectorTypeQRCode (which requires 10.10) in the constructor for
BarcodeDetectionImplMac. A test relies on being able to construct this
object even on pre-10.10 systems, so we cannot use the availability
attribute or assert that we are 10.10 or higher.

Bug:  735328 
Change-Id: I64883a34911b95e624b6ccb0f638034920aef75d
Reviewed-on: https://chromium-review.googlesource.com/565028
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485484}
[modify] https://crrev.com/58299c8150a3f7248a986eec7d87210a6abf0896/services/shape_detection/barcode_detection_impl_mac.mm

Project Member

Comment 31 by bugdroid1@chromium.org, Jul 11 2017

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

commit 8cc8696f5b264d23ace8f2ae6535518c213eabac
Author: Peter Collingbourne <pcc@chromium.org>
Date: Tue Jul 11 18:36:42 2017

Silence availability warning for call to operatingSystemVersion.

An upcoming version of clang removes the ability to suppress
availability warnings by redeclaring functions. The new way to suppress
warnings is to enclose the function reference in an "if (@available)"
block.

But we don't want to do that for this call because it would
require us to check for 10.10, as that was when the API was officially
documented, and we should try to avoid using Gestalt here wherever possible
because it has been observed to spin up threads among other things. 

So continue to use a respondsToSelector check here instead and silence
the warning.

Bug:  735328 
Change-Id: I3e20478a929fb3a18d2a1583a1fb4ddfddcb79ea
Reviewed-on: https://chromium-review.googlesource.com/564225
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485681}
[modify] https://crrev.com/8cc8696f5b264d23ace8f2ae6535518c213eabac/base/sys_info_mac.mm

Project Member

Comment 32 by bugdroid1@chromium.org, Jul 12 2017

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

commit fe2a747f216ca18ee1e46f9480caad3fe35300d0
Author: Peter Collingbourne <pcc@chromium.org>
Date: Wed Jul 12 01:44:49 2017

Add availability attributes to NSView function redeclarations.

An upcoming version of clang removes the ability to suppress
availability warnings by redeclaring functions. This means that the
references to the types NSPressureConfiguration, NSLayoutXAxisAnchor,
NSLayoutYAxisAnchor and NSLayoutDimension in our redeclarations will
cause fatal warnings when compiling with a 10.12 SDK and targeting
10.9 because these types are marked as unavailable in 10.9.

To avoid this, add availability attributes to the functions that
reference these types. According to Apple's documentation, the function
NSView::setPressureConfiguration was added in 10.11, not 10.10.3,
so move its declaration to the 10.11 block.

Because we have updated the availability for the functions that we
have redeclared, we must now switch to the new method for suppressing
availability warnings for references to those functions, which is to
either annotate the caller with an availability attribute, or enclose
the function reference in an "if (@available)" block. This patch
does so for all references to the functions that we have redeclared,
as well as for two enumerator references which the new compiler will
complain about.

Bug:  735328 
Change-Id: I5b447932d57cc9459a05a75050f4212892b66714
Reviewed-on: https://chromium-review.googlesource.com/564224
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485781}
[modify] https://crrev.com/fe2a747f216ca18ee1e46f9480caad3fe35300d0/base/mac/sdk_forward_declarations.h
[modify] https://crrev.com/fe2a747f216ca18ee1e46f9480caad3fe35300d0/chrome/browser/ui/cocoa/browser_window_touch_bar.mm
[modify] https://crrev.com/fe2a747f216ca18ee1e46f9480caad3fe35300d0/chrome/browser/ui/cocoa/framed_browser_window.mm
[modify] https://crrev.com/fe2a747f216ca18ee1e46f9480caad3fe35300d0/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm

Project Member

Comment 33 by bugdroid1@chromium.org, Jul 12 2017

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

commit 51938fff0607fee07eb38e896fcca6b262acf19e
Author: Peter Collingbourne <pcc@chromium.org>
Date: Wed Jul 12 01:45:55 2017

crdmg: Use __builtin_available/availability instead of -mmacosx-version-min.

We can't use @available in this file because that is only available
in Objective C.

Bug:  735328 
Change-Id: I09ed1b2bcc46fceafc4bfb84800d18177fd4f4a3
Reviewed-on: https://chromium-review.googlesource.com/566956
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Reviewed-by: Sidney San Martin <sdy@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485783}
[modify] https://crrev.com/51938fff0607fee07eb38e896fcca6b262acf19e/chrome/utility/safe_browsing/mac/BUILD.gn
[modify] https://crrev.com/51938fff0607fee07eb38e896fcca6b262acf19e/chrome/utility/safe_browsing/mac/crdmg.cc

Project Member

Comment 34 by bugdroid1@chromium.org, Jul 12 2017

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

commit 063509f9e490079536cc8cb019d50ab40a075f13
Author: Peter Collingbourne <pcc@chromium.org>
Date: Wed Jul 12 17:23:33 2017

Roll clang 305735:307486.

This includes a change to the compiler that removes the ability to
suppress availability warnings by redeclaring functions. The new way
to suppress warnings is to enclose the function reference in an
"if (@available)" block.

Previous CLs have cleaned up the code base to use the new way of
suppressing warnings (see bug).

Bug:  735328 
Change-Id: I340ac5c16df837d13fa5b8885e9f5a8e7bcf8e0c
Reviewed-on: https://chromium-review.googlesource.com/566280
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486004}
[modify] https://crrev.com/063509f9e490079536cc8cb019d50ab40a075f13/tools/clang/scripts/update.py

Project Member

Comment 35 by bugdroid1@chromium.org, Jul 12 2017

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

commit 3e629c165a838883cd338801810068a6fdcc5c6d
Author: Nico Weber <thakis@chromium.org>
Date: Wed Jul 12 18:49:25 2017

Disable --gdb-index in fuchsia builds for now.

fuchsia builds use lld by default, and it looks like the recent clang roll
made this "error" an actual error (meaning it makes the link fail) in lld.

So disable --gdb-index for now to get the fuchsia builds green. Don't disable
for use_lld in general so we don't forget to fix this before enabling
lld on linux.

Bug:  735101 , 735328 
Change-Id: Iadf2547af91d2c15290824ada6dddc5c15b40426
Reviewed-on: https://chromium-review.googlesource.com/568678
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486038}
[modify] https://crrev.com/3e629c165a838883cd338801810068a6fdcc5c6d/build/config/compiler/BUILD.gn

My local build with the 10.12 SDK has broken:

../../ui/base/cocoa/touch_bar_util.mm:24:11: error: 'setBezelColor:' is only available on macOS 10_12_1 or newer [-Werror,-Wunguarded-availability]
  [button setBezelColor:[NSColor colorWithSRGBRed:0.168

There is also:

../../content/browser/accessibility/browser_accessibility_cocoa.mm:579:8: error: 'NSAccessibilityRequiredAttribute' is only available on macOS 10_12 or newer [-Werror,-Wunguarded-availability]
      {NSAccessibilityRequiredAttribute, @"required"},

I'm currently at revision 2806adc6cae9df89a37e4ee7eca4ace525312afe.

Am I better off checking out a revision before 063509f9e490079536cc8cb019d50ab40a075f13 for now?
You should send a fix. The CLs mentioned above should have many examples. The 10.12 SDK doesn't currently have bot coverage, so we rely on devs fixing issues (Also see https://chromium-review.googlesource.com/c/570007/7 which sadly has been in the cq for way too long.)
Project Member

Comment 38 by bugdroid1@chromium.org, Jul 13 2017

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

commit 5c0cdc08290b127c8a3a060dd53b9f2627700bc2
Author: Nico Weber <thakis@chromium.org>
Date: Thu Jul 13 23:33:56 2017

mac: -Wpartial-availability cleanups.

- The canonical name of the warning is now -Wunguarded-availability,
  so use that
- Remove a spurious dupe of the flag in device/gamepad which caused
  the flag to be passed twice to compiles there
- Remove a redeclaration that's no longer needed

Bug:  735328 
Change-Id: I510c7b6f6835ef971df4fb746f612fdf875edc31
Reviewed-on: https://chromium-review.googlesource.com/570741
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Reviewed-by: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486536}
[modify] https://crrev.com/5c0cdc08290b127c8a3a060dd53b9f2627700bc2/build/config/compiler/BUILD.gn
[modify] https://crrev.com/5c0cdc08290b127c8a3a060dd53b9f2627700bc2/device/gamepad/BUILD.gn

Project Member

Comment 39 by bugdroid1@chromium.org, Jul 13 2017

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

commit 23f11eae0f75b87c853ec1383a34d6469b89be9d
Author: Nico Weber <thakis@chromium.org>
Date: Thu Jul 13 23:39:41 2017

mac: Use spiffy API_AVAILABILE macro instead of raw __attribute__

This is the recommended way to use the new @available feature.
Also happens to work around clang-format not understanding
__attribute__.

Bug:  735328 ,741084
Change-Id: I3f518b23037d2cdd23a5e32c0fc3de1f865bedd0
Reviewed-on: https://chromium-review.googlesource.com/570007
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486539}
[modify] https://crrev.com/23f11eae0f75b87c853ec1383a34d6469b89be9d/base/BUILD.gn
[add] https://crrev.com/23f11eae0f75b87c853ec1383a34d6469b89be9d/base/mac/availability.h
[modify] https://crrev.com/23f11eae0f75b87c853ec1383a34d6469b89be9d/base/mac/sdk_forward_declarations.h
[modify] https://crrev.com/23f11eae0f75b87c853ec1383a34d6469b89be9d/chrome/browser/app_controller_mac.mm
[modify] https://crrev.com/23f11eae0f75b87c853ec1383a34d6469b89be9d/chrome/browser/ui/cocoa/browser_window_touch_bar.mm
[modify] https://crrev.com/23f11eae0f75b87c853ec1383a34d6469b89be9d/chrome/browser/ui/cocoa/tabs/tab_strip_view.mm
[modify] https://crrev.com/23f11eae0f75b87c853ec1383a34d6469b89be9d/chrome/browser/ui/cocoa/tabs/tab_window_controller.h
[modify] https://crrev.com/23f11eae0f75b87c853ec1383a34d6469b89be9d/chrome/utility/safe_browsing/mac/crdmg.cc
[modify] https://crrev.com/23f11eae0f75b87c853ec1383a34d6469b89be9d/components/handoff/handoff_manager.h
[modify] https://crrev.com/23f11eae0f75b87c853ec1383a34d6469b89be9d/components/handoff/handoff_manager.mm

https://chromium-review.googlesource.com/#/c/571316/ fixes @avail with the 10.12 sdk.
Project Member

Comment 41 by bugdroid1@chromium.org, Jul 15 2017

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

commit 2d245df79351a09a300805f1d991376cc476ac68
Author: Erik Chen <erikchen@chromium.org>
Date: Sat Jul 15 03:16:06 2017

Update availability annotations for macOS 10.12 SDK.

The 10.12 SDK adds new availability annotations for many existing methods.
Existing code must be updated with availability annotations to prevent
"-Wunguarded-availability" warnings from triggering when compiling against the
10.12 SDK.

For more details, see
https://clang.llvm.org/docs/LanguageExtensions.html#objective-c-available

TBR=avi@chromium.org, davidben@chromium.org, reillyg@chromium.org

Bug:  669240 , 735328 
Change-Id: Ifebb4a6f5b7f2171f0d0ba732c92d8ac79bc435f
Reviewed-on: https://chromium-review.googlesource.com/571316
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486974}
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/app_controller_mac.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.h
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller_unittest.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller.h
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller_unittest.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/browser_window_touch_bar.h
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/browser_window_touch_bar.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/browser_window_touch_bar_unittest.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.h
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa_browsertest.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/framed_browser_window.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/l10n_util.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor_unittest.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/content/browser/accessibility/browser_accessibility_cocoa.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/net/ssl/ssl_platform_key_mac.cc
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/third_party/WebKit/Source/platform/fonts/mac/FontFamilyMatcherMac.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/ui/accelerated_widget_mac/BUILD.gn
[add] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/ui/accelerated_widget_mac/availability_macros.h
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/ui/accelerated_widget_mac/ca_layer_tree_coordinator.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/ui/accelerated_widget_mac/ca_layer_tree_unittest_mac.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/ui/accelerated_widget_mac/ca_renderer_layer_tree.h
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/ui/accelerated_widget_mac/ca_renderer_layer_tree.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/ui/base/cocoa/touch_bar_forward_declarations.h
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/ui/views/cocoa/bridged_content_view_touch_bar.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/ui/views/cocoa/bridged_native_widget.h
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/ui/views/cocoa/bridged_native_widget.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/ui/views/cocoa/drag_drop_client_mac_unittest.mm
[modify] https://crrev.com/2d245df79351a09a300805f1d991376cc476ac68/ui/views/cocoa/native_widget_mac_nswindow.mm

Project Member

Comment 42 by bugdroid1@chromium.org, Jul 17 2017

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

commit 6ae692d78280408674d5a66ec0ab21ca3bf3631e
Author: Nico Weber <thakis@chromium.org>
Date: Mon Jul 17 02:37:03 2017

mac: Use @available instead of -Wunguarded-availability suppression where possible.

This works despite https://llvm.org/PR33796 here because these
functions are in an unnamed namespace.

Bug:  735328 
Change-Id: Ia22c05303a0360573aa1a0385c6a3966767f84cd
Reviewed-on: https://chromium-review.googlesource.com/573113
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487023}
[modify] https://crrev.com/6ae692d78280408674d5a66ec0ab21ca3bf3631e/content/common/quarantine/quarantine_mac.mm
[modify] https://crrev.com/6ae692d78280408674d5a66ec0ab21ca3bf3631e/content/common/quarantine/quarantine_mac_unittest.mm

Comment 43 by h...@chromium.org, Jul 17 2017

Status: Started (was: Untriaged)
Bumping the status to "started".

Maybe we should mark it fixed since the roll's been in for a while?

Comment 44 by p...@chromium.org, Jul 17 2017

I was going to leave it open until the known remaining build problems on Mac caused by the roll were fixed.

thakis: is that the case?
They're not quite fixed yet. Things are fine with the 10.10 SDK. 10.12 SDK needs 2 more tweaks, 10.13 SDK yet 3 more in addition to that.
Status: Fixed (was: Started)
https://chromium-review.googlesource.com/c/574683/ fixed 10.12. 10.13 is still in beta.
Project Member

Comment 47 by bugdroid1@chromium.org, Jul 28 2017

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

commit 8f2b9e6f267c7fd2dccd55fc4ba95a4635251b89
Author: Nico Weber <thakis@chromium.org>
Date: Fri Jul 28 20:19:19 2017

net: Use @available instead of suppressing -Wunguarded-availability.

No intended behavior change. See
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/iZsW1jAsc2A
for motivation.

Bug:  735328 
Change-Id: I9dee0408c5b9cc60eeb2454f20d44b13cd4152ff
Reviewed-on: https://chromium-review.googlesource.com/592047
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Matt Mueller <mattm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490497}
[modify] https://crrev.com/8f2b9e6f267c7fd2dccd55fc4ba95a4635251b89/net/cert/x509_util_mac.cc

Project Member

Comment 48 by bugdroid1@chromium.org, Aug 14 2017

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

commit d09279c9b7cd06cc9d1fa91c03c3a49338674041
Author: Nico Weber <thakis@chromium.org>
Date: Mon Aug 14 19:41:31 2017

mac: Remove workaround for a compiler bug that's now fixed.

Bug:  669240 , 735328 , 753944 
Cq-Include-Trybots: master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
Change-Id: Ia478da385744cb064da270819322abc3d8789f9c
Reviewed-on: https://chromium-review.googlesource.com/614081
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494135}
[modify] https://crrev.com/d09279c9b7cd06cc9d1fa91c03c3a49338674041/services/shape_detection/barcode_detection_impl_mac.h
[modify] https://crrev.com/d09279c9b7cd06cc9d1fa91c03c3a49338674041/services/shape_detection/text_detection_impl_mac.h
[modify] https://crrev.com/d09279c9b7cd06cc9d1fa91c03c3a49338674041/ui/base/cocoa/touch_bar_util.h

Sign in to add a comment