Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(31)

Issue 635363002: Correct NSScreen comparison on PPAPI fullscreen switching (Closed)

Created:
4 years, 10 months ago by Jiang Jiang
Modified:
4 years, 10 months ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Correct NSScreen comparison on PPAPI fullscreen switching From OS X 10.10, NSApplicationDidChangeScreenParametersNotification will be sent when window enters fullscreen, and [NSScreen screens] will be updated with new NSScreen instances equal to the previous ones but with different pointer values. As a result, using == to compare pointers of desiredScreen and current menu bar/dock screen will break as the containsObject: check in -update does isEqual: comparison. This doesn't affect Chrome as Chrome is using EmbedsFullscreenWidget(). Committed: https://crrev.com/b31efc78ccea78706ebd298a5e82340cd563691f Cr-Commit-Position: refs/heads/master@{#298989}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -2 lines) Patch
M ui/base/cocoa/fullscreen_window_manager.mm View 1 chunk +5 lines, -2 lines 0 comments Download

Messages

Total messages: 27 (6 generated)
Jiang Jiang
Hi, Do you feel like looking at it? It's should be possible to reproduce with ...
4 years, 10 months ago (2014-10-08 15:29:20 UTC) #2
erikchen
I'll take a look today. Is there an associated bug?
4 years, 10 months ago (2014-10-08 16:24:53 UTC) #3
Jiang Jiang
On 2014/10/08 16:24:53, erikchen wrote: > I'll take a look today. Is there an associated ...
4 years, 10 months ago (2014-10-08 16:53:22 UTC) #4
erikchen
On 2014/10/08 16:53:22, Jiang Jiang wrote: > On 2014/10/08 16:24:53, erikchen wrote: > > I'll ...
4 years, 10 months ago (2014-10-08 21:14:36 UTC) #5
Jiang Jiang
On 2014/10/08 21:14:36, erikchen wrote: > On 2014/10/08 16:53:22, Jiang Jiang wrote: > > On ...
4 years, 10 months ago (2014-10-09 08:48:27 UTC) #6
Jiang Jiang
On 2014/10/09 08:48:27, Jiang Jiang wrote: > On 2014/10/08 21:14:36, erikchen wrote: > > On ...
4 years, 10 months ago (2014-10-09 13:11:35 UTC) #7
erikchen
lgtm
4 years, 10 months ago (2014-10-09 17:07:07 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/635363002/1
4 years, 10 months ago (2014-10-09 18:06:34 UTC) #10
erikchen
You're going to need an OWNER review. avi or rsesek is probably your best bet.
4 years, 10 months ago (2014-10-09 18:14:17 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/16701)
4 years, 10 months ago (2014-10-09 18:18:51 UTC) #13
Jiang Jiang
Avi, can you please take a look?
4 years, 10 months ago (2014-10-09 18:35:35 UTC) #16
Jiang Jiang
Avi, can you please take a look?
4 years, 10 months ago (2014-10-09 18:35:35 UTC) #17
Avi (use Gerrit)
Why do they do this kind of thing? I'm feeling grateful that at least -isEqual: ...
4 years, 10 months ago (2014-10-09 18:38:18 UTC) #18
erikchen
On 2014/10/09 18:38:18, Avi wrote: > Why do they do this kind of thing? I'm ...
4 years, 10 months ago (2014-10-09 18:43:44 UTC) #19
Jiang Jiang
On 2014/10/09 18:43:44, erikchen wrote: > On 2014/10/09 18:38:18, Avi wrote: > > Why do ...
4 years, 10 months ago (2014-10-09 19:10:33 UTC) #20
erikchen
On 2014/10/09 19:10:33, Jiang Jiang wrote: > On 2014/10/09 18:43:44, erikchen wrote: > > On ...
4 years, 10 months ago (2014-10-09 19:52:49 UTC) #21
Jiang Jiang
On 2014/10/09 19:52:49, erikchen wrote: > On 2014/10/09 19:10:33, Jiang Jiang wrote: > > On ...
4 years, 10 months ago (2014-10-09 21:50:13 UTC) #22
erikchen
Gotcha. Thanks for the detailed update! On Thu, Oct 9, 2014 at 2:50 PM, <jiangj@opera.com> ...
4 years, 10 months ago (2014-10-09 21:53:14 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/635363002/1
4 years, 10 months ago (2014-10-09 22:07:34 UTC) #25
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 10 months ago (2014-10-09 22:12:06 UTC) #26
commit-bot: I haz the power
4 years, 10 months ago (2014-10-09 22:12:49 UTC) #27
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/b31efc78ccea78706ebd298a5e82340cd563691f
Cr-Commit-Position: refs/heads/master@{#298989}

Powered by Google App Engine
This is Rietveld 408576698