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
(deleted), text/html
Details
HP Printer Identifier Plugin Version 4 (ZIP file)
(deleted), application/octet-stream
Details
Patch to fix the hang
(deleted), 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 (deleted) —
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
Attached patch Patch to fix the hang (deleted) — — Splinter Review
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: