Closed Bug 53451 Opened 24 years ago Closed 24 years ago

Following HP Plugin page causes a freeze in Seamonkey, not Communicator

Categories

(Core Graveyard :: Plug-ins, defect, P1)

Product:
Core Graveyard
Component:
Plug-ins
Platform:
x86
Windows NT
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: bijals, Assigned: serhunt)

References

(
URL
)

Details

(Whiteboard: [nsbeta3++][PDTP1])

Attachments

(3 files)

Actual test case
632 bytes, text/html
Details
HP Printer Identifier Plugin Version 4 (ZIP file)
29.72 KB, application/octet-stream
Details
Patch to fix the hang
2.36 KB, patch
Details | Diff | Splinter Review 
Steps:
1) Open the following page in Navigator using "Open Web Location ..." or "Open 
File ..."
(HTML page attached below)

Actual Results: Seamonkey Navigator freezes.

Expected Results: Page loads and does not freeze Navigator

Build Date/Platform: 2000092005 using NT
Attached file Actual test case — 

    
    

    
  
Nominating for nsbeta3 since this Web page is a part of the HP Plugin which is 
contractually required for Seamonkey.

    
    

    
  
Making it P1 since this plugin has to work in Seamonkey.
Keywords: nsbeta3
Priority: P3 → P1

    
    

    
  
cc: self

    
    

    
  
Okay, not sure why attachment did not work.  Take the HTML code below and save 
it in a blank document for testing:

<html>
<head>
<title>Preprint</title>
</head>
<body>

<script>
var sPath = location.href;
sPath = sPath.substring(0, sPath.lastIndexOf('/'));
var now = new Date();
var nextPage = sPath + '/PIVersion.html?x=' + now.getTime();
var embed = '<embed'
                + ' type="application/x-vnd.hp-hppi"'
                + ' NextPage="' + nextPage + '"'
                + ' IceCommand="GetPIVersion"'
                + ' SiteFriendlyName="test message: See plugin version."'
                + ' height="100"'
                + ' width="100"'
                + '>\n';

window.document.write(embed);
</script>
</body>



    
    

    
  
Did not freeze after I tried to load this html from my HD. (2000092108m18)

    
    

    
  
Marking nsbeta3+.  Reassigning to Clayton.

Assignee: asa → clayton
Whiteboard: [nsbeta3+]

    
    

    
  
Shrirang, I just tried with 2000092116 build and Seamonkey froze so this might
not be a consistent problem, but it's reproducible.


    
    

    
  
PDT:  Since contractual, marking ++ and P1.

Assignee: clayton → av
Whiteboard: [nsbeta3+] → [nsbeta3+][nsbeta3++][PDTP1]

    
    

    
  
I cannot repro. But my build is a bit older than both mentioned above. Pulling 
new tree right now...

    
    

    
  
Also note that the test case lacks closing html tag. Just observation.

    
    

    
  
Still cannot reproduce. Do I need to have an actual plugin installed?

    
    

    
  
Okay, I tested some more and verified that the crash occurs only when the 
GetPIVersion.html page is loaded with the HP plugin installed.  I will include 
the plugin as an attachment so that we can test it.  

I plan to setup a conference call with HP to figure out what is causing Netscape 
6 to freeze Tuesday or Wednesday.

    
    

    
  
correcting the status whiteboard to show only "[nsbeta3++]".  When PDT marks a 
bug with ++, we usually just add an extra + to the [nsbeta3+] already shown.

Thanks.
Whiteboard: [nsbeta3+][nsbeta3++][PDTP1] → [nsbeta3++][PDTP1]

    
  
Group: netscapeconfidential?

    
    

    
  
The situation looks as follows. When the plugin is invoked Mozilla tries to send 
it a stream with just an HTML source which started the plugin. It always does it 
for every plugin. So plugins should be ready for that and consume the stream, 
discarding the data if not needed. This particular plugin does not expect the 
stream and returns zero on NPP_WriteReady meaning it is not ready/willing 
to consume the stream which results in indefinite looping because Mozilla thinks  
just that and keeps trying.

And here I see two issues:
1. Is this unsolicited stream good behaviour by Mozilla? I think yes.
2. We probably don't want to use the simple for loop when NPP_WriteReady returns 
zero to keep trying. We should at least yield to other tasks every cycle to 
avoid consuming all the processor power and hanging.

Adding some plugin-friendly people to the cc list.
Status: NEW → ASSIGNED

    
    

    
  
Shouldn't we just break out of the loop if the plugin is not ready after, oh,
maybe 100,000 times?


    
    

    
  
Andrei, why is it good behavior to send this initial stream to the plugin? If
Nav4 doesn't do it (and rpresumably it does not or the plugin would hang there
too), then why should Mozilla?


    
    

    
  
Because it does not contradict the spec. But plugins according to the spec 
should return zero if there is no memory resources or allocation not ready or 
something like that, but not when they just don't want the stream.

    
    

    
  
Giving up after some number of attempts also came to my mind. And it could be 
configurable via hidden prefs.

    
    

    
  
OK, I tried one million, it took about ten seconds on my 450 MHz machine with 
debug build. Looks like good default number. Do we want to go this way?

    
    

    
  
Proposed changes (just schematic, not real diffs):

+   PRUint32 attemptcount = 0;
    while (amountRead > 0)
    {
      numtowrite = CallNPP_WriteReadyProc(...);
      if ((numtowrite > amountRead))
        numtowrite = amountRead;
      if(numtowrite > 0)
      {
        writeCount = CallNPP_WriteProc(...);
        amountRead -= numtowrite;
      }
+     else if(numtowrite == 0)
+     {
+       attemptcount++;
+       if(attemptcount >= 1000000)
+         return NS_ERROR_FAILURE;
+     }
    }


    
    

    
  
I think 10 seconds is too long, especially since it blocks the UI thread. If we
can yield, then it might be OK, but if not I'd consider getting it closer to two
seconds. This is 'how long do we wait for the plugin to be ready to get data'
not 'how long before we get data from the net', so I cannot see any reason to
wait so long (personal opinion only).

The code example you provided looks good: simple and low-risk. A timer might be
a better long-term solution, but this seems right for now. How about 200,000
instead of 1,000,000 though?


    
    

    
  
Sounds reasonable. Should we check prefs for the number too? Say, zero 
or negative having a special meaning, e.g. don't give up at all?

    
    

    
  
I would also recommend 2 seconds since it freezes up the UI and makes Seamonkey 
look broken.

    
    

    
  
Deos this mean we need another version of the plugin or is this a client side
change?


    
    

    
  
No, I think the plugin should work as expected. By the way, what is it supposed 
to do? Does it have GUI? I could not see anything with attached testcases even 
on 4.x.

    
    

    
  
*spam*
moving to plug-ins, we should keep browser general clean

Status: ASSIGNED → NEW
Component: Browser-General → Plug-ins
QA Contact: doronr → shrir

    
    

    
  
If you go to a special HP site, the plugin will display a web page asking if the 
site can read your printer information which is used to upsell other printer 
products.

End user is given a optin dialog before any information is read.

    
    

    
  
Hi all,
I'm the plugin developer.  Feel free to contact me 
robertw@cv.hp.com, 541-715-2818.

The hppi plugin returns information to the website about printers that are 
available to your PC.  It does not have a UI.  It does not create or request a 
stream.  

In answer to some specific questions:

I concur with the statement that this is a new problem.  The plugin worked on an 
earlier version of PR2.  Someone wiped the disk on the test machine that had 
that code.

The stream being passed in is the page that invoked the plugin.  At the point 
the stream is opened by the browser  (NPP_NewStream) the plugin has already had 
a destroy call (NPP_Destroy).  The destroy call came because the plugin asked to 
leave in a getUrl call.  It has already concluded it's business and sent the 
requested data back to the website with 
NPN_GetUrl(instance, nextpage, _self)

Nextpage is passed in to the plugin, intended to be the page on the website that 
will use the printer information.  The data is passed back in search parameters:
An example nextpage:
http://cvpbdev2.cv.hp.com/PItest/PIVersion.html?piversion=1%2e0%2e0%2e5&mode=deb
ug

It's not clear to me why any plugin would want the invoking page passed to it in 
a new text stream.  Can you explain this in terms other than "it doesn't violate 
the spec"?

I apologise for the test page foulup.  The server I pointed you to had the 
network card die or something.  I'll post a new test site here shortly.

Thank you for your attention to this. 

Robert Williams    robertw@cv.hp.com    541-715-2818
HP in Corvallis Oregon

    
    

    
  
OK, got a test site outside the 'ol firewall.  Take your hppi enabled browser to 

http://hppew2.consonus.com/PItest/pi_gen.html

-robertw

    
    

    
  
What am I supposed to see when I click on one of the links? Currently I see 
nothing happening on 4.7
Status: NEW → ASSIGNED

    
    

    
  
You need to put the plugin files in the NS plugins directory.  All 3 of them.
-robertw

    
    

    
  


  

    
    

    
  
I just tried to turn off this initial stream and this broke RealAudio.

nhart: do you in fact rely on this?

    
    

    
  
OK, looks like there is not much sense to keep trying, since we are in the main 
message loop and thus the plugin doesn't have the room to breath anyway. So 
functionally equivalent solution would be to break away after the first attempt. 
I think this is what we need to do now and maybe file a bug on not trying to 
deliver data later as the spec says, or even just make the spec not say so, as 
the situation when plugin needs time to allocate buffer looks unlikely.

So schematically it will look like this:

    while (amountRead > 0)
    {
      numtowrite = CallNPP_WriteReadyProc(...);
+     if(numtowrite <= 0)
+       return NS_ERROR_FAILURE;

      if ((numtowrite > amountRead))
        numtowrite = amountRead;
-     if(numtowrite > 0)
      {
        writeCount = CallNPP_WriteProc(...);
        amountRead -= numtowrite;
      }
    }
I've already got r=waterson and warren

    
    

    
  
I've checked it into the branch. Will update the trunk later today.

    
  
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED

    
    

    
  
Marking fixed.

    
    

    
  
verified that the page does not hang anymore in today's windows br build 
2000092908. Adding keyword: vtrunk.
URL: http://hppew2.consonus.com/PItest/pi_...
Keywords: vtrunk

    
    

    
  
What is vtrunk? It is in the trunk too.

    
    

    
  
'vtrunk' keyword is newly added. It is for mozilla QA to verify this fix on the 
trunk since nscp QA is working on the branch right now. So I have not marked 
this as 'verified'

    
    

    
  
So, is this workaround/fix checked in for the commercial build which will be for 
RTM?  If no, we need to check this in both places post PR3 since it's late now.

    
    

    
  
Commercial builds pick up whatever is build in the Mozilla builds.

    
    

    
  
Cool, so the nsbeta3 branch has this change which will be in for PR3 so 
obviously RTM?

    
    

    
  
I checked it in in _both_ the branch _and_ the trunk. If this is what you are 
asking.

    
    

    
  
this does not cause a freeze/hang in win32 mozilla trunk build 100204 on NT with
HPplugin installed.  Setting Bug Status to Verified and removing the vtrunk keyword.

Status: RESOLVED → VERIFIED
Keywords: vtrunk

    
  
Product: Core → Core Graveyard

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: