533891 - remove default plugins

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Josh Aas]

I don't think we need default plugins any more. We should remove all of the default plugins and simply have display modes in the object frame - the object frame can just paint a missing plugin image when it doesn't have a plugin to display. We can add another display mode for a crashed plugin when we have OOP plugins.

Comment 1

[Josh Aas]

Attachment: fix v0.2

Comment 2

[Ryan VanderMeulen [:RyanVM]]

I suppose this takes care of bug 261751 too, yes?

Comment 3

[Benjamin Smedberg]

We want this same thing for plugin-crashed UI. Josh says he wants to revise this patch to remove the nsObjectFrame if there isn't an available plugin and use a PFS-style binding mechanism even when PFS isn't enabled.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Should this be handled inside nsObjectFrame, or should it be a different frame class combined with new pseudo-classes driven by nsObjectLoadingContent?

Comment 5

[Josh Aas]

(In reply to comment #4)
> Should this be handled inside nsObjectFrame, or should it be a different frame
> class combined with new pseudo-classes driven by nsObjectLoadingContent?

I have been discussing that with jst, we're probably going to do this from nsObjectLoadingContent, not nsObjectFrame.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Good. :-)

Comment 7

[Boris Zbarsky [:bzbarsky]]

Sounds good to me too.

Note that once we no longer have a default plug-in, we can possibly remove some hackery in nsObjectLoadingContent::OnStartRequest and certainly any consumers of nsObjectLoadingContent::ShouldShowDefaultPlugin (or rather any codepaths that follow on that returning true).

Comment 8

[Josh Aas]

Attachment: kill default plugins, v1.0

This patch simply kills off default plugins and their associated handling code.

Comment 9

[Josh Aas]

Attachment: kill default plugins, v1.1

Comment 10

[Nick Thomas [:nthomas] (UTC+12)]

Please update browser/installer/{package-manifest.in,removed-files.in} for files that are deprecated.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Also: please remove plugin.default_plugin_disabled from firefox.js when this lands.

Comment 12

[Josh Aas]

Attachment: fix v1.2

Updated to current mozilla-central. Does not remove the actual default plugin files to keep the patch manageable.

Comment 13

[Josh Aas]

Attachment: fix v1.3

Includes fixes for nsObjectLoadingContent.

Comment 14

[Josh Aas]

Attachment: fix v1.4

added entries to removed-files.in

Comment 15

[Johnny Stenback (:jst)]

Comment on attachment 443963 [details] [diff] [review]
fix v1.4

Yay! Glad to see this mess go! :)

Comment 16

[Ryan VanderMeulen [:RyanVM]]

One question I ran into when I was working on bug 261751 was whether or not the lack of a plugins/ directory out of the box would potentially cause breakage. Does the installer need to create it if it doesn't exist?

Comment 18

[Josh Aas]

I got this error on a Windows try server build, it appears to be a known intermittent orange (bug 453969) but I'm noting it here in case it is relevant in the future. I did not get the error on the Linux unit test run.

7363 INFO Running /tests/content/base/test/test_bug382113.html...
NEXT ERROR 7364 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug382113.html | Child got load event - got false, expected true
7365 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug382113.html | Object got load event - got false, expected true

Comment 19

[Justin Dolske [:Dolske]]

Comment on attachment 443963 [details] [diff] [review]
fix v1.4

>-ifeq ($(OS_ARCH),OS2)
>-TOOL_DIRS += default/os2
>-endif

Nit: Might want to ping the OS/2 folks to see if whatever gets built for OS/2 needs added to the removed-files list.

Comment 20

[Josh Aas]

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/e074757a15aa

Comment 21

[Nick Thomas [:nthomas] (UTC+12)]

The changes here result in no firefox/plugins directory any more, which causes xpcshell (Xo & Xd on linux, at least) to fail like this

bash -c cp bin/xpcshell firefox && cp -R bin/components/* firefox/components/ && cp -R bin/plugins/* firefox/plugins/ && python -u xpcshell/runxpcshelltests.py --symbols-path=symbols --manifest=xpcshell/tests/all-test-dirs.list firefox/xpcshell
cp: cannot create regular file `firefox/plugins/': Is a directory

which is a really poor error message for 'firefox/plugins doesn't exist'. If we still support loading out of plugins/ then it would make sense to leave an empty directory there, and it would make the automation happy.

(This didn't show up on the original push because the build system doesn't clobber dist/firefox before packaging, so it took a clobber to get off the relbranch from Seamonkey's tagging to make it show up.)

Also, why didn't browser/installer/package-manifest.in get updated ?

Comment 22

[Phil Ringnalda (:philor)]

Backed out.

Comment 23

[Josh Aas]

Attachment: fix v1.5

Includes fix for 'package-manifest.in'.

Comment 24

[[e-mail] (:bhearsum)]

(In reply to comment #21)
> which is a really poor error message for 'firefox/plugins doesn't exist'. If we
> still support loading out of plugins/ then it would make sense to leave an
> empty directory there, and it would make the automation happy.

There was some discussion about this in #developers and josh/bsmedberg decided against it. We'll work around this in Buildbot in the dependent bug.

Comment 25

[Josh Aas]

Pushed to mozilla-central again now that bug 564987 is fixed.

http://hg.mozilla.org/mozilla-central/rev/558df3e5b094

Comment 26

[Josh Aas]

Had to back out again due to a talos master problem. Should be fixed by releng tomorrow.

http://hg.mozilla.org/mozilla-central/rev/dd3bac33fac2

Comment 27

[Josh Aas]

pushed to mozilla-central again after releng fixes applied

http://hg.mozilla.org/mozilla-central/rev/6a9d0521a319

Comment 28

[Serge Gautherie (:sgautherie)]

Remnants:
http://mxr.mozilla.org/mozilla-central/search?string=default_plugin_disabled&case=1
Found 3 matching lines in 3 files

Comment 29

[Nick Thomas [:nthomas] (UTC+12)]

Also need to fix up 
 http://mxr.mozilla.org/mozilla-central/source/toolkit/toolkit-makefiles.sh#338
for this:
creating modules/plugin/default/mac/Makefile
can't read /builds/slave/mozilla-central-linux-nightly/build/modules/plugin/default/mac/Makefile.in: No such file or directory
creating modules/plugin/default/unix/Makefile
can't read /builds/slave/mozilla-central-linux-nightly/build/modules/plugin/default/unix/Makefile.in: No such file or directory
creating modules/plugin/default/windows/Makefile
can't read /builds/slave/mozilla-central-linux-nightly/build/modules/plugin/default/windows/Makefile.in: No such file or directory

Comment 30

[(not currently active) Ted Mielczarek]

There's a longstanding blanket-r+ from the build config peers to make obvious changes to allmakefiles.sh/toolkit-makefiles.sh.