store the os arch in pluginreg.dat, make sPluginHostImpl::ReadPluginInfo() return failure if OS arch doesn't match, bump plugin registry version from 0.08 to 0.09

RESOLVED WORKSFORME

Status

()

Core
Plug-ins
RESOLVED WORKSFORME
12 years ago
a year ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Assigned: Robert Sayre)

Tracking

({regression})

Trunk
x86
Mac OS X
regression
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.8.1.x +

Firefox Tracking Flags

(Not tracked)

Details

store the os arch in pluginreg.dat, make sPluginHostImpl::ReadPluginInfo() return failure if OS arch doesn't match, bump plugin registry version from 0.08 to 0.09

to fix bug #356694 ("Flash doesn't work on Intel Macs, problems with "Flash Player Enabler.plugin" (which is PowerPC)"), we are going to the second part of sayrer's fix for bug #346954 ("feed MIME types trigger plugin reloads")

josh and I were chatting and this is another approach.

bumping the version will cause nsPluginHostImpl::ReadPluginInfo() to fail (the first time), and if the stored archictecture of the pluginreg.dat doesn't match current OS arch, we'd fail as well.

if we do that, we could check back in sayrer's second part of the fix for bug #346954, because once the os arch and plugin version match, we can rely on the plugin cache.

http://lxr.mozilla.org/seamonkey/source/modules/plugin/base/src/nsPluginHostImpl.cpp#205

comments?
(Assignee)

Comment 1

12 years ago
(In reply to comment #0)
> 
> comments?
> 

If josh and jst are ok with the risk, I would prefer this.

Comment 2

12 years ago
I'd rather not do this on the 1.8 branch. That patch we're backing out doesn't do anything terribly important and its perfectly safe.
(Assignee)

Comment 3

12 years ago
This resulted in a totally unacceptable perf hit in 2.0.0.1. 

Backing out part of bug #346954 seems like the culprit.
Flags: blocking1.8.1.2?
Keywords: regression

Comment 4

12 years ago
(In reply to comment #3)
> This resulted in a totally unacceptable perf hit in 2.0.0.1. 
> 
> Backing out part of bug #346954 seems like the culprit.

We've had the original behavior since long before the patch that was initally landed, so I don't see how it could be an "unacceptable" perf hit. Gaining performance by doing something incorrectly should not up the bar for perf requirements.
(Assignee)

Comment 5

12 years ago
(In reply to comment #4)
> 
> We've had the original behavior since long before the patch that was initally
> landed, so I don't see how it could be an "unacceptable" perf hit.

Simple. We load plugin libs every time we visit a feed. We didn't used to have feeds.

> Gaining performance by doing something incorrectly 

Gaining anything by doing something incorrectly is bad. The pluginreg.dat format is flawed, just like all of the legacy "keep it simple" file formats we have.

> should not up the bar for perf requirements.

I am not suggesting that we have to make trade off. We should fix the edge case of PPC-to-Intel profile imports and not sniff the same libraries over and over, every time we visit a feed.
(Assignee)

Comment 6

12 years ago
(In reply to comment #5)
> 
> We should fix the edge case
> of PPC-to-Intel profile imports and not sniff the same libraries over and over,
> every time we visit a feed.

After making sure that the library sniffing is the cause of the perf hit, of course. It is a likely suspect.

Comment 7

12 years ago
Let's figure out the right thing to do that is safe for the 1.8 branch (just fix the edge case?).  Assigning to Josh for further investigation.
Assignee: nobody → joshmoz
Flags: blocking1.8.1.2? → blocking1.8.1.2+

Updated

12 years ago
Flags: blocking1.8.1.2+

Updated

12 years ago
Flags: wanted1.8.1.x+
(Assignee)

Comment 8

12 years ago
Josh, 

Are you going to have time for this in the foreseeable future? If not, I will take it.
(Assignee)

Comment 9

12 years ago
taking per IRC discussion w/ josh
Assignee: joshmoz → sayrer

Comment 10

a year ago
We do this.
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.