further limit Flash logic for deciding whether or not to run OOP

RESOLVED FIXED

Status

()

Core
Plug-ins
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Josh Aas, Assigned: Josh Aas)

Tracking

Trunk
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

2.75 KB, patch
BenWa
: review+
Josh Aas
: approval2.0+
Details | Diff | Splinter Review
(Assignee)

Description

7 years ago
Created attachment 513021 [details] [diff] [review]
fix v1.0

We have some code that is intended to force Flash < 10.1 to run in-process. We also have some code to make Flash run in-process on GMA9xx GPU machines. I don't think we need any of it, it just complicates things and actually makes the experience for users. I think this code was pushed before we had fully felt out our strategy for OOPP on Mac OS X.

In 64-bit mode its not worth ever trying to run Flash in-process in either of those situations. It won't work, and worse, it'll stop us from detecting Carbon negotiation and offering a 32-bit restart because we'll just fail loading the library.

On Mac OS X 10.5 we don't run anything OOP and we support Carbon in-process so it doesn't matter there.
Attachment #513021 - Flags: review?(b56girard)
I agree with the GMA9xx case and 64-bit case. 

I'm not clear however on what the outcome of running Flash < 10.1 OOP? (Also we've seen several minor release of 10.1 have various problems, so more generally Flash < Lastest). For example if a user is running ff 32-bit has Flash version 10.1 running OOP with the GMA9xx asking for carbon how will we handle that? Will it prompt to restart IP?
(Assignee)

Comment 2

7 years ago
If a user was running Carbon-negotiating Flash (for whatever reason, the version or the GPU) in a 32-bit browser on 10.6 it would just fail. No in-process restart offered. When I wrote this patch I didn't think this case was worth worry about but I'm having second thoughts. If we are worried about that case we could just wrap the Flash version check in an i386 ifdef.
(Assignee)

Comment 3

7 years ago
Created attachment 513056 [details] [diff] [review]
alternative fix v1.0

This version just limits all of the Mac OS X checks to i386, including the OS version check since that could only matter for i386 (64-bit must be 10.6+). I haven't even tried to compile this yet, I'm going to sleep on it.
(Assignee)

Updated

7 years ago
Attachment #513021 - Flags: review?(b56girard)
(Assignee)

Updated

7 years ago
Summary: get rid of Flash logic for deciding whether or not to run OOP → further limit Flash logic for deciding whether or not to run OOP

Comment 4

7 years ago
FWIW, I had suggested removing this code based on the understanding that in 32-bit mode, the default was not to run anything OOP on the Mac. Looking at the prefs file though, it looks like the default in 32-bit mode (on 10.6) specifically opts Flash into OOP mode, so I no longer think the first option here is a good idea.
(Assignee)

Updated

7 years ago
Attachment #513056 - Flags: review?(b56girard)

Updated

7 years ago
Attachment #513056 - Flags: review?(b56girard) → review+
(Assignee)

Updated

7 years ago
Attachment #513021 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Attachment #513056 - Flags: approval2.0+
(Assignee)

Comment 5

7 years ago
pushed to mozilla-central

http://hg.mozilla.org/mozilla-central
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
http://hg.mozilla.org/mozilla-central/rev/b5a403b079d3
(Assignee)

Comment 7

7 years ago
Ha, I should pay more attention to my copy/paste! Thanks Benoit.

Updated

7 years ago
Duplicate of this bug: 601669
You need to log in before you can comment on or make changes to this bug.