New issue
Advanced search Search tips

Issue 687822 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug

Blocked on:
issue 682516

Blocking:
issue 610589



Sign in to add a comment

Rename all clashes with Objective-C YES / NO keywords

Project Member Reported by patricia...@chromium.org, Feb 2 2017

Issue description

There are a bunch of instances in the Chrome codebase where "YES" or "NO" are defined, but this clashes with Objective-C's keywords for "true" and "false". Previously, this hasn't been a problem because these files have never been used in the same scope as Objective-C.

In order to refactor gfx::NativeViewAccessible to always be defined to the same thing on macOS (currently it is defined to "void*" or "id" depending on whether __OBJC__ is defined), we would like to include <objc/objc.h> (see https://opensource.apple.com/source/objc4/objc4-371.1/runtime/objc.h) in native_widget_types.h. The include then cascades down to many other files, causing the compiler to fail on identifier clashes on Mac.

To fix, we should rename all of these instances (probably they are all enums) to not use "YES" or "NO".
 
I think we should still fix this, but to get unblocked, perhaps try

typedef struct objc_class *Class;
typedef struct objc_object {
    Class isa;
} *id;

in place of #include <objc/objc.h> and ensure everything downstream works with that (i.e. ensure that this solves any linking problems we have for AXPlatformNode::FromNativeViewAccessible(gfx::NativeViewAccessible accessible)

If there are still problems we may need to look for other solutions (i.e. still have id typedef to void* in C++ code, and overload)
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 3 2017

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

commit edc842d64c63e7d820589a19d778a7af6355d64c
Author: patricialor <patricialor@chromium.org>
Date: Fri Feb 03 07:06:37 2017

Rename WebApplicationCacheHostImpl::IsNewMasterEntry enum to not use YES/NO.

Avoid clashes with YES/NO keywords in Objective-C.

BUG= 687822 

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

[modify] https://crrev.com/edc842d64c63e7d820589a19d778a7af6355d64c/content/child/appcache/web_application_cache_host_impl.cc
[modify] https://crrev.com/edc842d64c63e7d820589a19d778a7af6355d64c/content/child/appcache/web_application_cache_host_impl.h

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 6 2017

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

commit df7ed49e332e5ec80d670e6cc855722d8e368c7c
Author: patricialor <patricialor@chromium.org>
Date: Mon Feb 06 00:34:39 2017

Rename ShouldUpdateHistoryResult enum values to not use NO.

Avoid clashes with the NO keyword in Objective-C.

BUG= 687822 

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

[modify] https://crrev.com/df7ed49e332e5ec80d670e6cc855722d8e368c7c/chrome/browser/download/download_history.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 7 2017

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

commit 58397dcb1baf7977349645311d2e9d8c6a593375
Author: patricialor <patricialor@chromium.org>
Date: Tue Feb 07 08:42:50 2017

Rename encrypted media browsertest enum to not use YES/NO.

Avoid clashes with the YES/NO keywords in Objective-C.

BUG= 687822 

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

[modify] https://crrev.com/58397dcb1baf7977349645311d2e9d8c6a593375/chrome/browser/media/encrypted_media_browsertest.cc

Status: Fixed (was: Started)

Sign in to add a comment