Last Comment Bug 533891 - remove default plugins
: remove default plugins
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9.3a5
Assigned To: Josh Aas
:
: Benjamin Smedberg [:bsmedberg]
Mentors:
: 261751 (view as bug list)
Depends on: 486570 564987 703888
Blocks: 261751 519541 564656
  Show dependency treegraph
 
Reported: 2009-12-09 18:01 PST by Josh Aas
Modified: 2011-11-19 08:29 PST (History)
24 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix v0.2 (263.17 KB, patch)
2009-12-09 18:15 PST, Josh Aas
no flags Details | Diff | Splinter Review
kill default plugins, v1.0 (263.37 KB, patch)
2010-01-06 11:16 PST, Josh Aas
no flags Details | Diff | Splinter Review
kill default plugins, v1.1 (266.13 KB, patch)
2010-01-06 12:40 PST, Josh Aas
no flags Details | Diff | Splinter Review
fix v1.2 (20.00 KB, patch)
2010-05-06 13:47 PDT, Josh Aas
no flags Details | Diff | Splinter Review
fix v1.3 (22.87 KB, patch)
2010-05-06 14:07 PDT, Josh Aas
no flags Details | Diff | Splinter Review
fix v1.4 (24.83 KB, patch)
2010-05-06 14:28 PDT, Josh Aas
dolske: review+
jst: superreview+
Details | Diff | Splinter Review
fix v1.5 (25.60 KB, patch)
2010-05-10 22:10 PDT, Josh Aas
no flags Details | Diff | Splinter Review

Description Josh Aas 2009-12-09 18:01:36 PST
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 2009-12-09 18:15:55 PST
Created attachment 416857 [details] [diff] [review]
fix v0.2
Comment 2 Ryan VanderMeulen [:RyanVM] 2009-12-09 19:30:05 PST
I suppose this takes care of bug 261751 too, yes?
Comment 3 Benjamin Smedberg [:bsmedberg] 2009-12-10 14:38:57 PST
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-10 2010-01-05 11:39:13 PST
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 2010-01-05 12:20:25 PST
(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-10 2010-01-05 13:58:02 PST
Good. :-)
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2010-01-05 21:09:35 PST
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 2010-01-06 11:16:05 PST
Created attachment 420377 [details] [diff] [review]
kill default plugins, v1.0

This patch simply kills off default plugins and their associated handling code.
Comment 9 Josh Aas 2010-01-06 12:40:02 PST
Created attachment 420391 [details] [diff] [review]
kill default plugins, v1.1
Comment 10 Nick Thomas [:nthomas] 2010-01-10 17:56:11 PST
Please update browser/installer/{package-manifest.in,removed-files.in} for files that are deprecated.
Comment 11 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-04-12 14:05:16 PDT
Also: please remove plugin.default_plugin_disabled from firefox.js when this lands.
Comment 12 Josh Aas 2010-05-06 13:47:47 PDT
Created attachment 443946 [details] [diff] [review]
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 2010-05-06 14:07:32 PDT
Created attachment 443955 [details] [diff] [review]
fix v1.3

Includes fixes for nsObjectLoadingContent.
Comment 14 Josh Aas 2010-05-06 14:28:03 PDT
Created attachment 443963 [details] [diff] [review]
fix v1.4

added entries to removed-files.in
Comment 15 Johnny Stenback (:jst, jst@mozilla.com) 2010-05-06 15:32:37 PDT
Comment on attachment 443963 [details] [diff] [review]
fix v1.4

Yay! Glad to see this mess go! :)
Comment 16 Ryan VanderMeulen [:RyanVM] 2010-05-06 17:16:45 PDT
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 17 Ryan VanderMeulen [:RyanVM] 2010-05-06 17:17:31 PDT
*** Bug 261751 has been marked as a duplicate of this bug. ***
Comment 18 Josh Aas 2010-05-06 17:42:10 PDT
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] 2010-05-10 16:38:10 PDT
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 2010-05-10 18:44:01 PDT
pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/e074757a15aa
Comment 21 Nick Thomas [:nthomas] 2010-05-10 21:26:05 PDT
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) 2010-05-10 21:46:26 PDT
Backed out.
Comment 23 Josh Aas 2010-05-10 22:10:28 PDT
Created attachment 444595 [details] [diff] [review]
fix v1.5

Includes fix for 'package-manifest.in'.
Comment 24 Ben Hearsum (:bhearsum) 2010-05-11 12:55:28 PDT
(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 2010-05-13 11:56:42 PDT
Pushed to mozilla-central again now that bug 564987 is fixed.

http://hg.mozilla.org/mozilla-central/rev/558df3e5b094
Comment 26 Josh Aas 2010-05-13 13:18:11 PDT
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 2010-05-14 10:12:17 PDT
pushed to mozilla-central again after releng fixes applied

http://hg.mozilla.org/mozilla-central/rev/6a9d0521a319
Comment 28 Serge Gautherie (:sgautherie) 2010-05-20 20:43:37 PDT
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] 2010-05-20 21:08:40 PDT
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 Ted Mielczarek [:ted.mielczarek] 2010-05-21 05:15:10 PDT
There's a longstanding blanket-r+ from the build config peers to make obvious changes to allmakefiles.sh/toolkit-makefiles.sh.

Note You need to log in before you can comment on or make changes to this bug.