Closed
Bug 159445
Opened 22 years ago
Closed 21 years ago
Scan for PLIDs upon startup or load time (for plugins)
Categories
(Core Graveyard :: Plug-ins, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.4final
People
(Reporter: arun, Assigned: peterlubczynski-bugs)
References
Details
(Keywords: fixed1.4)
Attachments
(1 file, 2 obsolete files)
32.75 KB,
patch
|
john
:
review+
alecf
:
superreview+
asa
:
approval1.4+
|
Details | Diff | Splinter Review |
Bug 44973 makes mention of PLIDs in tags like OBJECT (and perhaps EMBED) that gives plugin authors a way to invoke plugins uniquely, without fear of MIME type collisions among popular types. But this bug raises a different aspect of PLIDs on Windows. Before mentioning the bug, the "specifications" are: http://mozilla.org/projects/plugins/first-install-problem.html and http://mozilla.org/projects/plugins/plugin-identifier.html . PLIDs on Windows are registry entries in the Win32 System Registry. When the browser scans for plugins in its own plugins directories ("plugins" directory for Mozilla, for example), it should also add the plugins directories specified under the PLID keys. In other words, it should also scan for plugins that were installed into separate directories before Gecko-based browser even arrived on the desktop. It should scan HKEY_LOCAL_MACHINE and the MozillaPlugins subkey, then look for the Path and XPTPath strings, and make sure it knows where to find these components. It can also build MIME type information from the other strings written to the registry in the spec. (http://mozilla.org/projects/plugins/first-install-problem.html) The distinction: Bug 44973 mentions tags such as OBJECT using PLID, but this bug specifically mentions a startup scan of the registry to add further plugins directories to the list of existing plugins directories.
Updated•22 years ago
|
Priority: -- → P2
Whiteboard: [PL2:P2][Plid][Plug-in Mgr]
Target Milestone: --- → mozilla1.2beta
Comment 1•22 years ago
|
||
moving to 1.4 beta : plugin branch work
Assignee: beppe → anthonyd
Whiteboard: [PL2:P2][Plid][Plug-in Mgr] → [PL:BRANCH][Plid][Plug-in Mgr]
Target Milestone: mozilla1.2beta → mozilla1.4beta
Assignee | ||
Comment 2•22 years ago
|
||
*** Bug 164980 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•21 years ago
|
Status: NEW → ASSIGNED
QA Contact: shrir → bmartin
Assignee | ||
Comment 4•21 years ago
|
||
okay, here's a patch that finds plugins based on the information left in their PLID Windows Registry key. I've tested with Real and Viewpoint and both seemed to be correctly "scanned". There are several limitation: 1) You either get ALL the plugins specified by PLIDs or none. Users or embedders can toggle this with the new pref in winpref.js: plugin.scan.plid.all (default is true) 2) XPT paths doesn't work. Bug 44973 is blocking this and looks like a lot of work. 3) Any plugins found in the directory specified by the Path key will be found. If the Path key points to a filename, the entire directory will still be scanned. Our system currently only has the ability to scan a whole directory vs. just one DLL. For example, when Real One is scanned, it not only picks up nppl3260.dll which is only what the Path key specifies, but also nprjplug.dll and nprpjplug.dll which are in the same directory. 4) PLID and their Path keys are scanned in the order they appear in the Windows registry. 5) PLID Path scanning happens after embedder specified directories but before scanning teir1 plugin directories (like QT, WMP, Acrobat, Java, etc..). This is important for dups and resolving mime-type conflicts. 6) Other information from the PLID key, like mime type, is completely ignored. All the mime and description information is read directly from the DLL. Plugins will be choosen based on the order in which the mime type appears in about:plugins.
Assignee | ||
Updated•21 years ago
|
Whiteboard: [PL:BRANCH][Plid][Plug-in Mgr]
Assignee | ||
Comment 5•21 years ago
|
||
patch ready for reviews
Attachment #122169 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #123188 -
Flags: superreview?(alecf)
Attachment #123188 -
Flags: review?(dougt)
Comment 6•21 years ago
|
||
Comment on attachment 123188 [details] [diff] [review] patch v.1.1 + for (PRInt32 i = 0; i < dirs.Count(); i++) { don't call Count() every time - save the value and reuse it. + nsCOMPtr<nsIFile> dup (do_QueryInterface(dirs[i])); why do_QueryInterface? this should work without even casting, no? (or NS_STATIC_CAST if you must) - then "dup" can be a raw nsIFile, and you can avoid the extra addref/release + mPrefService = do_GetService(NS_PREFSERVICE_CONTRACTID); + if (mPrefService) { what's the point of caching the prefs service if you're going to call do_GetService() every time :) Make sure you'd not leaking the pref service though. In any case, is it really even worth caching the whole service? you should only cache it if you expect to be calling methods on it often - this looks like initialization, mostly (except perhaps that security.enable_java?) // This table controls the order of scanning const char *prefs[] = {NS_WIN_JRE_SCAN_KEY, nsnull, NS_WIN_ACROBAT_SCAN_KEY, nsnull, this isn't part of your patch, but if you feel like making this "const char* const prefs[]" I'll sr= that too :) sr-'ing this for now - nothing major but there were enough changes to recommend a new patch.
Attachment #123188 -
Flags: superreview?(alecf)
Attachment #123188 -
Flags: superreview-
Attachment #123188 -
Flags: review?(dougt)
Assignee | ||
Comment 7•21 years ago
|
||
okay, corrected the patch according to comments.... I left in mPrefService because the old code would fetch it every time whereas this patch will cache it in the plugin host service constructor and release it when the service is told to destory (when Notify/Observe(quit-application) happens). The method I'm adding the new pref check to is |FindPlugins| which is called frequently enough to cache the service, plus the prefs service is fetched multiple times in scanning and starting plugins anyway. The DOMPluginImpl::GetFilename method still call the service every time but I'm looking forward to re-writing this code in the next milestone anyway.
Attachment #123188 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #123213 -
Flags: superreview?(alecf)
Attachment #123213 -
Flags: review?(dougt)
Comment 8•21 years ago
|
||
Comment on attachment 123213 [details] [diff] [review] patch v.1.2 ok, this all looks good except usually we like to release cached services during xpcom shutdown (NS_XPCOM_SHUTDOWN_OBSERVER_ID rather than quit-application - xpcom-shutdown is a better-defined topic (i.e. its kind of a frozen topic, rather than quit-application which may or may not fire in embedded environments) other than that, sr=alecf.. thanks for cleaning up the pref table
Attachment #123213 -
Flags: superreview?(alecf) → superreview+
Assignee | ||
Updated•21 years ago
|
Attachment #123213 -
Flags: review?(dougt) → review?(jkeiser)
Assignee | ||
Comment 9•21 years ago
|
||
*** Bug 154890 has been marked as a duplicate of this bug. ***
Comment 10•21 years ago
|
||
Comment on attachment 123213 [details] [diff] [review] patch v.1.2 Patch is simpler than it looks :) r=me
Attachment #123213 -
Flags: review?(jkeiser) → review+
Assignee | ||
Comment 11•21 years ago
|
||
Comment on attachment 123213 [details] [diff] [review] patch v.1.2 seeing approval from drivers: this fix is needed so that mozilla can find plugins that were installed before the browser
Attachment #123213 -
Flags: approval1.4?
Comment 12•21 years ago
|
||
Comment on attachment 123213 [details] [diff] [review] patch v.1.2 a=asa (on behalf of drivers) for checkin to 1.4.
Attachment #123213 -
Flags: approval1.4? → approval1.4+
Assignee | ||
Comment 13•21 years ago
|
||
I've opened bug 207306 to track cleaning up all cached services in the plugin host on the correct observer topic. patch checked into trunk and 1.4 branch, marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Target Milestone: Future → mozilla1.4final
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•