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

Issue 666893 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 0
Type: Bug



Sign in to add a comment

MacViews: Crash when a dialog appears on a Mac with a Touch Bar

Project Member Reported by sdy@chromium.org, Nov 18 2016

Issue description

Version: 56.0.2924.0
OS: macOS 10.12.1

What steps will reproduce the problem?
(1) Make sure you're using a Mac with a Touch Bar, or are running the Touch Bar simulator in Xcode, and MacViews dialogs are enabled.
(2) Trigger a dialog (e.g. alert() or basic auth).

What is the expected result?
The dialog appears, and ideally buttons appear on the Touch Bar, too.

What happens instead?
Crash:
+[NSObject groupItemWithIdentifier:items:]: unrecognized selector sent to class 0x7fffb31af140
 

Comment 1 by sdy@chromium.org, Nov 18 2016

Components: Internals>Views
Labels: Proj-MacViews
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 21 2016

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

commit 799f1635ddedf7fd2f69f7dca26b0f2bf6399cba
Author: sdy <sdy@chromium.org>
Date: Mon Nov 21 16:33:06 2016

[Mac] Switch from NSObject categories to forward declarations for Touch Bar support.

This fixes a crash when -[BridgedContentView touchBar:] calls
-[NSGroupTouchBarItem groupItemWithIdentifier:items:], because
NSGroupTouchBarItem was typedef'd to NSObject.

BUG= 666893 

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

[modify] https://crrev.com/799f1635ddedf7fd2f69f7dca26b0f2bf6399cba/ui/base/cocoa/touch_bar_forward_declarations.h
[modify] https://crrev.com/799f1635ddedf7fd2f69f7dca26b0f2bf6399cba/ui/views/cocoa/bridged_content_view_touch_bar.mm

Comment 3 by sdy@chromium.org, Nov 21 2016

Status: Fixed (was: Started)

Comment 4 by mmoss@google.com, Nov 22 2016

Cc: gov...@chromium.org
Labels: -Pri-3 M-57 Pri-0
Status: Assigned (was: Fixed)
I believe the fix in #2 is causing a failure on the Mac official builder:

https://luci-milo.appspot.com/buildbot/official.desktop/mac64/1266

../../ui/base/cocoa/touch_bar_forward_declarations.h:17:1: error: unknown type name 'NS_ASSUME_NONNULL_BEGIN'
NS_ASSUME_NONNULL_BEGIN
^

Comment 5 by gov...@chromium.org, Nov 22 2016

Cc: manoranj...@chromium.org mmohammad@chromium.org

Comment 6 by sdy@chromium.org, Nov 22 2016

The issue seems to be that this builder is running Xcode 5, and those macros were introduced in Xcode 6. Any idea why it's different?

Comment 7 by gov...@chromium.org, Nov 22 2016

Mac official Build failure has been tracked under bug 667830.

Comment 8 by sdy@chromium.org, Nov 22 2016

Status: Fixed (was: Assigned)
I'm going to close this because erikchen@ is working on a proper fix in bug 667830. Feel free to reopen it if that isn't right.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 23 2016

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

commit ad8fde91632a1de9759cf18404c6755d815c48aa
Author: tapted <tapted@chromium.org>
Date: Wed Nov 23 05:42:29 2016

Revert of [Mac] Switch from NSObject categories to forward declarations for Touch Bar support. (patchset #4 id:100001 of https://codereview.chromium.org/2519563002/ )

Reason for revert:
Persistent build failures on official.desktop (see http://crbug.com/667830)

Breaks compilation with the 10.12.1 SDK:
bridged_content_view_touch_bar.mm:50:4: error: 'NSTouchBarItem' is partial: introduced in macOS 10.12.1 [-Werror,-Wunguarded-availability]

Also tapted doesn't like the macros.

Original issue's description:
> [Mac] Switch from NSObject categories to forward declarations for Touch Bar support.
>
> This fixes a crash when -[BridgedContentView touchBar:] calls
> -[NSGroupTouchBarItem groupItemWithIdentifier:items:], because
> NSGroupTouchBarItem was typedef'd to NSObject.
>
> BUG= 666893 
>
> Committed: https://crrev.com/799f1635ddedf7fd2f69f7dca26b0f2bf6399cba
> Cr-Commit-Position: refs/heads/master@{#433558}

TBR=sdy@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 666893 , 667830

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

[modify] https://crrev.com/ad8fde91632a1de9759cf18404c6755d815c48aa/ui/base/cocoa/touch_bar_forward_declarations.h
[modify] https://crrev.com/ad8fde91632a1de9759cf18404c6755d815c48aa/ui/views/cocoa/bridged_content_view_touch_bar.mm

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 23 2016

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

commit db32364af64c0e4fb64b6fb1df7b4e7a23b44811
Author: sdy <sdy@chromium.org>
Date: Wed Nov 23 18:44:22 2016

Revert of [Mac] Switch from NSObject categories to forward declarations for Touch Bar support. (patchset #4 id:100001 of https://codereview.chromium.org/2519563002/ )

Reason for revert:
Persistent build failures on official.desktop (see http://crbug.com/667830)

Breaks compilation with the 10.12.1 SDK:
bridged_content_view_touch_bar.mm:50:4: error: 'NSTouchBarItem' is partial: introduced in macOS 10.12.1 [-Werror,-Wunguarded-availability]

Also tapted doesn't like the macros.

Original issue's description:
> [Mac] Switch from NSObject categories to forward declarations for Touch Bar support.
>
> This fixes a crash when -[BridgedContentView touchBar:] calls
> -[NSGroupTouchBarItem groupItemWithIdentifier:items:], because
> NSGroupTouchBarItem was typedef'd to NSObject.
>
> BUG= 666893 
>
> Committed: https://crrev.com/799f1635ddedf7fd2f69f7dca26b0f2bf6399cba
> Cr-Commit-Position: refs/heads/master@{#433558}

TBR=sdy@chromium.org,tapted@chromium.org
BUG= 666893 , 667830
NOTRY=true
NOPRESUBMIT=true

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

Review-Url: https://codereview.chromium.org/2530633002
Cr-Commit-Position: refs/branch-heads/2929@{#4}
Cr-Branched-From: d78075be67a3ee6520d1f8377cd69b478ebdf5ee-refs/heads/master@{#434071}

[modify] https://crrev.com/db32364af64c0e4fb64b6fb1df7b4e7a23b44811/ui/base/cocoa/touch_bar_forward_declarations.h
[modify] https://crrev.com/db32364af64c0e4fb64b6fb1df7b4e7a23b44811/ui/views/cocoa/bridged_content_view_touch_bar.mm

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 30 2016

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

commit 3a6a6c13fe021aa3124ecaadf4655aa76ce47a99
Author: sdy <sdy@chromium.org>
Date: Wed Nov 30 23:36:32 2016

[Mac] Switch from NSObject categories to forward declarations for Touch Bar support.

This fixes a crash when -[BridgedContentView touchBar:] calls
-[NSGroupTouchBarItem groupItemWithIdentifier:items:], because
NSGroupTouchBarItem was typedef'd to NSObject.

This is an improved version of
https://crrev.com/799f1635ddedf7fd2f69f7dca26b0f2bf6399cba

BUG= 666893 

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

[modify] https://crrev.com/3a6a6c13fe021aa3124ecaadf4655aa76ce47a99/ui/base/cocoa/touch_bar_forward_declarations.h
[modify] https://crrev.com/3a6a6c13fe021aa3124ecaadf4655aa76ce47a99/ui/views/cocoa/bridged_content_view_touch_bar.mm

Sign in to add a comment