124936 - Duplicate plugin dll names cause refresh loop on navigator.plugins.refresh(1)

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[av (gone)]

I am filing this bug mainly not to forget to check this issue on the trunk after  
fix for 119621 goes in. Similar fix for the 0.9.4 branch caused this kind of 
problem and is filed in bugscape report 11998. So, the present bug can be 
reproduced and verified using steps from there. Alternatively, the following 
should do on Windows:

1. make sure you have, say, nppdf32.dll in _both_ Mozilla and 4.x plugins 
folders

2. load a simple html containing navigator.plugins.refresh(1) directive in its 
body (I am going to attach one)

3. see continous reload

Basically, the problem is that our refresh loop prevention mechanism which 
involves calculating the plugins check sum to detect changes before refreshing 
plugins and reloading the page, does not take into account the fact that there 
could be plugin dlls with the same name in different directories.

Comment 1

[av (gone)]

Attachment: Test case

Comment 2

[av (gone)]

Attachment: patch v.1

I added a method |nsPluginHostImpl::IsDuplicatePlugin| which is used to
determine that a plugin in question (when we scan plugin folders) is in fact
the same as we already found in some previous folder and already added to our
plugin list. It is treated as an 'unwanted' plugin in all aspects: it gets
cached in the plugin info list, it is not added to the 'loaded' plugin list,
changes to it are detected during refresh/rescan.

Please review.

Comment 3

[Peter Lubczynski]

Comment on attachment 69140 [details] [diff] [review]
patch v.1

r=peterl

Comment 4

[av (gone)]

|nsPluginHostImpl::IsDuplicatePlugin| might seem confusing as on the first 
glance one may think that on Mac only leaf name are compared, but in fact if 
leaf names are the same it goes further and compares the full pathes too (if 
they are not the same, we already got what we need and no further comparison 
needed), so Mac is fine.

Comment 5

[av (gone)]

To clarify the reason for this bug. If we have dup plugins they are not stored 
in our plugin info cache. This causes our change detection mechanism think that 
they have just been added thus resulting in endless refresh loop on 
plugins.refresh(1). I made them added to cache so they now act just like 
'unwanted' plugins and do not scew up change detection code.

Comment 6

[TaylorToddK]

Just a quick question, didn't get to really look at the patch very well.  If 
this patch weeds out duplicates, does it take into account version and and 
timestamp diffs?  In other words, is it possible the first plug-in loaded is 
not the latest version and the latest version of the plug-in will get cached as 
an unwanted plugin?

Comment 7

[Patrick C. Beard]

Comment on attachment 69140 [details] [diff] [review]
patch v.1

sr=beard

Comment 8

[av (gone)]

Todd: the patch does not change the behaviour in sense of what plugin is going 
to be picked up as a plugin to handle the content. It just fixes the glitch in 
the change detection code. If the version is present in the plugin name field in 
the version stamp the plugin will not be considered duplicate even having the 
same file name. Look at |nsPluginTag::Equals| to see how we compare plugins and 
decide when they are the same. The precedence is still determined by creation 
time (if in the same folder) or directory scan sequence (if in different 
folders).

Comment 9

[av (gone)]

Checked in. Marking fixed.

Shrirang, would you please also verify it with the steps from Bugscape 11998?

Comment 10

[shrirang khanzode]

sure. 

Comment 11

[shrirang khanzode]

verified with steps from bugscape bug, things look great.verified on 0812 trunk.