Closed Bug 53363 Opened 24 years ago Closed 24 years ago

PDF Support Problem: NPN_RequestRead is not implemented

Categories

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

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.1

People

(Reporter: lmcquarr, Assigned: peterlubczynski-bugs)

References

()

Details

(Keywords: helpwanted, perf, Whiteboard: wanted for 0.9.1 Patch ready - need review.)

Attachments

(9 files)

9/20 Win Build. This bug is related to bug 53344. Mozilla is supposed to tell us when a stream is seekable (server says "Accept-Ranges: bytes") at NPN_NewStream time. It isn't, but I made an experimental plug-in where I assumed it did so that I could continue testing byte range request support. 1 - First, In order to verify this bug, you will need to either fix 53344, or get a copy of my experimental plug-in. 2 - Start acrobat and go to FILE, PREFERENCES, GENERAL, and un check the check box marked "Allow Background Downloading". 3 - Start mozilla and go to http://access.adobe.com/cookbook.pdf 4 - After about 90K of download, Acrobat will issue an NPN_RequestRead for a bogus range. This is to kill background downloading (I am going to enter a separate bug about this undocumented feature, but basically, Acrobat and Netscape engineers worked out this deal where Netscape keeps pouring the file to Acrobat from the initial GET **until** the very first NPN_ReadRequest from Acrobat, which will then kill that first read -- that's background downloading in Acrobat.). Expected result: A read request like the following (a snapshot of the logfile I captured of the HTTP conversation between Nav 4.73 and the access.adobe.com server): GET http://access.adobe.com/cookbook.pdf HTTP/1.0 Proxy-Connection: Keep-Alive User-Agent: Mozilla/4.73 [en] (Windows NT 5.0; U) Range: bytes=0-1,2-3 Request-Range: bytes=0-1,2-3 Host: access.adobe.com Accept: image/gif, image/x-xbitmap, image/jpeg, image/pjpeg, image/png, */* Accept-Encoding: gzip Accept-Language: en Accept-Charset: iso-8859-1,*,utf-8 Notice the bogus byte range request: Request-Range: bytes=0-1,2-3 Mozilla, however, never issues the read though I have set a breakpoint in the Acrobat plug-in and I can see that we are issuing the read. How bad is this, you wonder? Well, once again, if you look at bug 53344, you can see that a lot of work has been done by Adobe and by Netscape and by Microsoft (Mac and Win IE) to support byte range requests so that Acrobat works well on the Web. Without this feature, I don't think I can recommend that we modify our installers to put the nppdf.dll plug-in into the Nav 6 plug-ins directory. We didn't do it for Mac IE 4.5, because it did not support byte range requests. Mac IE 5.0 does, however.
Keywords: acrobat, nsbeta3
Priority: P3 → P2
This bug is a dup of 53365 (I think I got impatient with your server and committed the bug twice).
*** Bug 53365 has been marked as a duplicate of this bug. ***
If I understand you correctly, this negatively affects the performance of PDF files viewed in Netscape 6 but doesn't prevent them from being viewed. That would likely place this into the category of bugs that Netscape unfortunately lacks the resources to fix for Netscape 6 RTM given the limited time remaining. Marking [nsbeta3-] and Future for now on that basis; if I've misunderstood, please clear [nsbeta3-] from the Status Whiteboard and correct my misunderstanding. Liz, is there any chance that you or another Adobe engineer could sign up to implement the fix? If you will, and it's considered low risk when reviewed by Mozilla reviewers, I could argue for accepting the patch on an exception basis because Adobe is a high profile partner and Acrobat is the #2 plug-in on the web per Hotbot.
Keywords: helpwanted, perf
Whiteboard: [nsbeta3-]
Target Milestone: --- → Future
Erik -- I am asking my management now. I am not hopeful, however, since they only begrudged me an afternoon to do this testing (and I have already spent a day). Feel free to try to sweet talk Sarah Rosenbaum, however.
Hi Eric: Unfortunately, but as I suspected, my management does not want to spend their also limited resources right now on this issue. That means that not I (or any one else on the Acrobat team) can work on byte range support in mozilla right now.
Setting bug status to New. Bugs whaich have been triaged to a target milestone should not be Unconfirmed.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: 4xp
Adding rtm+.
Keywords: rtm
Whiteboard: [nsbeta3-] → [nsbeta3-] [rtm+]
*** Bug 32319 has been marked as a duplicate of this bug. ***
PDT marking [rtm-] based on ekrock's comments
Whiteboard: [nsbeta3-] [rtm+] → [nsbeta3-] [rtm-]
Blocks: 55959
Blocks: 53372
Setting mozilla0.8
Target Milestone: Future → mozilla0.8
Updated Keywords. Replace "nsbeta3" and "rtm" with "nsbeta1".
Nominating as a high-priority bug for nsbeta1. This is one of the bugs in plug- in API backward compatibility of most concern to Adobe.
Keywords: nsbeta1
Target Milestone: mozilla0.8 → mozilla0.9
Will the new netlib support this?
Whiteboard: [nsbeta3-] [rtm-]
necko does not generate byte range requests on its own, but it should be able to allow byte range requests from smart clients such as plugins. gagan: do you know of an outstanding bug for byte range support?
Blocks: 74980
Gagan, Byte Range support is VERY important to Adobe Acrobat (as lmcquarr@adobe.com points out) and Acrobat is very important to our embedding vendors. Do you have any hints on how to implement this? Thanks!
darin: AFAIK plugins are the only consumer for byte-ranges. Though cache could be next-- we could add checks for partial entries (aborted/disconnected) and try byte-ranges on them. Peter: we really don't have to do anything inside necko to use byte-ranges. The only thing required is for plugins to specify the range on the HTTP channel with a SetRequestHeader call with the appropriate parameters for the byte-range (see nsIHTTPChannel.idl)
Inorder to make byte-range requests work, you'd need to tell HTTP to not use the cache. This can be done by setting the FORCE_RELOAD flag on the HTTP channel.
Updating the URL field.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Oops, closed accidentally, reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
Reporter: I have a fix for 'never setting stream seekable'. So NPP_NewStream informs Acrobat that the stream is in fact seekable, and after it returns the stream type is successfully set to NP_SEEK. But after that I never hit a break point in _requestread as you mentioned in your original report. Could you please confirm that RequestRead is actually expected?
I justs filed bug 76356 on setting byte range support info for the stream. It look like stand alone issue and can be reviewed and checked in separately. Accordingly, changing summary and platform for this bug.
OS: Windows 2000 → All
Hardware: PC → All
Summary: PDF Support Problem: No Byte Range Request → PDF Support Problem: NPN_RequestRead is not implemented
Whiteboard: eta -- 4.17.01
Whiteboard: eta -- 4.17.01
-->0.9.1
Target Milestone: mozilla0.9 → mozilla0.9.1
The patch (id=31081) is checked in so plugins are now aware of the stream seekability and can behave accordingly, i.e. issue RequestRead calls. We are at least unblocked for testing and further development. Couple of more large doc testcases: http://partners.adobe.com:80/asn/developer/acrosdk/docs/fdftkref.pdf http://partners.adobe.com:80/asn/developer/acrosdk/docs/coreapi/CoreAPIReference .pdf http://partners.adobe.com:80/asn/developer/acrosdk/docs/extapi/FormsAPIReference .pdf Peter, to enable this feature in Acrobat you should go to its preferences and clear 'Allow Background Downloading' checkbox.
Attached patch some ugly patchSplinter Review
With the latest patch I can eventually see the following function called: nsPluginStreamInfo::RequestRead(nsByteRange* rangeList) { return NS_ERROR_NOT_IMPLEMENTED; } Looks like with current API design all further action should happen from here.
Actually, the feature is enabled by default in Acrobat. Clearing the "allow background downloading" checkbox will just ensure that you will be able to hit that RequestRead API with Acrobat more quickly. To hit the RequestRead API with 100% certainty, disable background downloading, and then go to one of the large PDF files at the above URLs and start paging through the file. ("Background downloading" means we attempt to just let the entire file pour down in the background if possible, so that if the user wants to go to some other page, that page is already on his/her hard drive and doesn't require a hit to the network. If background downloading is disabled, which I recommend that you do for your testing, then will kill the initial connection when the first page is downloading. So for every other page, we will issue the RequestRead with muliple byte ranges. )
In this patch I tried to put some code which is supposed to create/cancel channels and send byte range requests to the http server. The patch is not a fix, it is very raw and needs tuning as well as a lot of polishing. Right decisions on when to send requests, when to instruct the listener to cancel the request and so on are yet to be made.
The following typo must be corrected in the attacment 31691: static char * makeByteRangeString(nsByteRange* aRangeList) { ... // XXX zero length? - if(range->length) + if(range->length == nsnull) continue;
Blocks: 77775
A question from Andrei and an answer. First the question: " Another thing, particularly Gagan (our network guy) may want to know to fix the multiple range bug (and you might want to comment on this in that new bug report). When you issue multiple range request, are you expecting every data range come separately (maybe in multiple but still separate for every range NPP_Write's) or, say, two ranges may come in one data chunk with some sort of delimeter? In other words who parses the incoming data for ranges?" The API for NPP_Write is int32 NP_LOADDS NPP_Write(NPP instance, NPStream *stream, int32 offset, int32 len, void *buffer). It allows for ***only one*** range to be written to the plug-in at a time, and that is what the Acrobat plugin expects. One the byte range data comes from the server, the browser needs to parse it just enough to create this NPP_Write data structure and to pass the data on. On-the-other-hand, the browser can handle the incoming data in whatever fashion is the cleanest for it. At least one browser that supports this byte range API (Mac IE 5) is passing us byte ranges that have no correlation to the ranges that come from the server (see my notes about tiny ranges from Mac IE 5 causing us problems in bug 77775). Would this be useful to you folks? What if I came up with my project files and sources and sat down with anyone who is interested, and we stepped through the Acrobat plug-in in action in the debugger with Navigator 4.x? I could even do that today. Let me know.
Priority: P2 → P1
>>------- Additional Comments From Darin Fisher 2001-04-06 14:56 ------- >>Inorder to make byte-range requests work, you'd need to tell HTTP to not use >>the cache. This can be done by setting the FORCE_RELOAD flag on the HTTP >>channel. From Darin's post, it looks like Necko can do byte-range requests as long as this flag is set. Is there example of anything actually using this currently in Mozilla? This would let us do everything in one stream? Is there any dependency on a Necko bug?
Necko can do a single range request with a minor glitch -- in OnDataAvailable it doesn't set the aSourceOffset parameter, it is always zero. But we need multiple range request as Acrobate uses it. I filed a bug on this. Another thing is that we cannot do this in one stream (what is a stream anyway? Request? Channel?). Looks like we need a wrapper to pretend that we are one stream for the plugin.
Andrei, Can you link up the dependecy bugs if you get a chance? Thanks!
No longer blocks: 77775
Moving 77775 in the dependency tree from 'being blocked' to 'blockers'.
Depends on: 77775
I don't see this geting fxied by 0.9.1 moving to 0.9.2.
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Depends on: 78741
dougt, clayton asked me to reassign this to you. Do you have time?
Assignee: av → dougt
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.2 → mozilla0.9.1
Last night I implemented segmented byte ranges in necko - yawn. Now I am just trying to figure out the right thing to do in the plugin.
Status: NEW → ASSIGNED
Woohoo! Thanks Doug!
Doug, if you need help from the plugin side, let me know.
So, I cleaned up Av's patch and now am sending the requested byteranges through the plugin API. However, I am seeing the PDF reader just stall after the last requested byte is sent. After this point, the content UI is empty and responsiveness drops. I tried viewing the same PDF in a trunk build, and the results were similar. However, with the trunk build, the PDF initially display. I am using version version 4.05c. Are there other known issues that I should know about?
It it useful for me to take a look at this at this point and tell you what I see from the plug-in side? If so, will the patch be in the daily build?
In all likelihood what is happening is this: When Acrobat requests ranges, it sits in a loop waiting for them to arrive. In some cases, this is a blocking read, and in some cases, it is a non-blocking read for Acrobat. In all cases, however, if Acrobat requested some bytes ranges, it NEEDS them and really can't do much without them. It will wait five minutes and then time out with an error messages. I see from this bug that the range request NPN_RequestRead is now implemented. Does that mean you are also calling us back (NPP_Write) with the ranges we requested? If you are calling us back with the ranges and Acrobat is still hanging, then I think there is a problem with the ranges that are being returned. I can help you debug this, but I need to know exactly what is implemented at this point.
Doug, update your nglsrc and/or nsObjectFrame.* as I checked in a NP_WRITE buffer curruption fix last night. That could solve your problem. Does the full-page PDF viewer work without your patch? If not, if you see, just the gray backgroud of the content viewer, I bet it's part of the regression caused by Hyatt's checkin. He did check in a patch last week, so perhaps all you need to do is update. I've kinda seen this randomly with his patch, but haven't been able to consistantly reproduce, especially in commercial bits. Also, you may want to try using the PDF plugin with the EMBED or OBJECT instead of full-screen if THAT is blocking you. Please let me know as if full-page PDF doesn't work, that needs to be addressed by beta.
Last patch is everything so far. If you update your *entire* tree and rebuild, then apply my patch, range requests will start limping for you. For whatever reason, it still does not work entirely correct. What is happening is that the first patch immediatly displays. When I try to click on the scroll bar, it just freezes. I am fairly confident that I am sending the right bytes. Peter, can you give this a spin?
the patch also fixes 81654
Good news: The last patch make RequestRange() work. :-) Not so good news: A request of a range 0-x may not work. So, if you have to request from byte zero to something, that may fail. A little more tooling and this can be fixed. However I need to balance my load, and this bug is not as serious as the other on my plate right now. IMO, this problem can be filed and latered for another milestone. Right now, I need a couple reviewers and some testing from anyone interested. Peter, I need a r= from you as soon as possible. Who does sr= on the plugin module?
Whiteboard: Patch ready - need review.
Doug, I'm having trouble building with the patch. CL is complaining about ns4xPluginStreamListener.h and I can't find it anywhere.
sorry bout that. I thought I did a 'cvs diff -uN'
I still can not get it to build. I'll try again at work, but carefully reading over the full-context HTML diffs, this looks like the right thing to do. r=peterl Marc, can you sr= this?
I'm not going to be able to sr this until others can build and run the patch - my first question has to be 'how was this tested and are you sure it is not leaking?' To that end, it looks like mDataForwardToRequest is allocated (via new) but never freed... Please request the sr again when this has been leak-checked.
Cool, thanks Doug. Peter and I played with it and it looks nice. I'm assuming you have tried it on Linux (at least to build)? sr=attinasi
Fix checked in: Checking in nglsrc/ns4xPlugin.cpp; /cvsroot/mozilla/modules/plugin/nglsrc/ns4xPlugin.cpp,v <-- ns4xPlugin.cpp new revision: 1.48; previous revision: 1.47 done Checking in nglsrc/ns4xPluginInstance.cpp; /cvsroot/mozilla/modules/plugin/nglsrc/ns4xPluginInstance.cpp,v <-- ns4xPluginInstance.cpp new revision: 1.51; previous revision: 1.50 done Checking in nglsrc/ns4xPluginInstance.h; /cvsroot/mozilla/modules/plugin/nglsrc/ns4xPluginInstance.h,v <-- ns4xPluginInstance.h new revision: 1.17; previous revision: 1.16 done RCS file: /cvsroot/mozilla/modules/plugin/nglsrc/ns4xPluginStreamListener.h,v done Checking in nglsrc/ns4xPluginStreamListener.h; /cvsroot/mozilla/modules/plugin/nglsrc/ns4xPluginStreamListener.h,v <-- ns4xPluginStreamListener.h initial revision: 1.1 done Checking in nglsrc/nsPluginHostImpl.cpp; /cvsroot/mozilla/modules/plugin/nglsrc/nsPluginHostImpl.cpp,v <-- nsPluginHostImpl.cpp new revision: 1.255; previous revision: 1.254 done Checking in public/nsIPluginStreamListener.h; /cvsroot/mozilla/modules/plugin/public/nsIPluginStreamListener.h,v <-- nsIPluginStreamListener.h new revision: 1.6; previous revision: 1.5 done Checking in npsimple.cpp; /cvsroot/mozilla/modules/plugin/test/npsimple.cpp,v <-- npsimple.cpp new revision: 1.34; previous revision: 1.33 done
Status: ASSIGNED → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → FIXED
Thank you so much for this work. I had trouble building with the patch, so I had to wait until this patch was checked in to do testing, which I am doing this morning. I am seeing some major problems. Should I log a new bug with the details?
More details on my previous comments. The major problem is that Mozilla is sending out the byte ranges in the **incorrect format**. The server, therefore, is not returning the bytes that we need. (The cookbook.pdf example just happens to work because of serendipity.) The range format is supposed to be: <start of range> - <end of range>, <start of range> - <end of range>, etc A correct example: Range: bytes=1594992-1596035, 1649050-1651239, 1024926-1025427, 1025428-1026639, 1026640-1029191 Here is what mozilla is sending out: Range: bytes=15949921596035-, 16490501651239-, 10249261025427-, 10254281026639-, 10266401029191- Hopefully this is a trivial fix!
How did the Cookbook example even work? Are there some other examples we can test with that will show us a) it is broken now and b) it is working when fixed? I'm reopening this based upon lmcquarr' comments.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I will put together about five very specific tests for you folks on an external server (it will take me about 45 minutes). In the meantime, our QA God Jeff Moran did send Shrirang a pointer to a test suite that he put on that external server a few months ago. (See below for info on how to access). Please note: for this bug, you should disable background downloading in Acrobat to more easily trigger byte range requests. You can do this in the prefs: General, Options, then uncheck "Allow Background Downloading". Date: Mon, 25 Sep 2000 14:07:42 -0700 To: shrir@netscape.com From: Jeff Moran <jemoran@adobe.com> Subject: Testing Net6 with Acrobat 4.05c Cc: dina,liz,mcshur,srosenba Shrirang Greetings. I work in Acrobat Quality Engineering. My manager has asked that I forward to you a simplified test suite for testing Acrobat 4.05c with Netscape 6.0. If you need information on how to get the most current version of Acrobat just let me know. http://access.adobe.com/browser/netscape/net6.pdf This PDF file contains all the necessary information, background terms and concepts, that you will need to test Netscape 6 with Acrobat 4.05c. It also contains links to test files that you may need. Please review it and direct any questions about it to me. We also have a few acrobat forms test suites already on access.adobe.com. You should include these in your testing as well. 1 - Ping form. http://access.adobe.com/browser/pingform.pdf When you press submit, the returned FDF is displayed in the spot where you can type. Note: no http headers should be displayed. 2 - Work Shop Statement http://access.adobe.com/browser/wkshp_statement_hardcoded.pdf Enter the number of pages and you should get back a PDF file that has values filled in for the fields. 3 - Catalog http://access.adobe.com/browser/form1.pdf Check the check boxes and press submit and it will bring back a catalog There are currently several bugs that affect viewing PDF files in Netscape 6. There are three areas that we consider major problems. They are (1) Byte range requests are currently not working [Bug 53365] , (2) Basic Acrobat forms cause an immediate crash in NPN_PostURL [Bug 53347] and (3) the Print Menu item is not hooked up to the NPN_Print API [Bug 53349] . One of our Engineers has logged these bugs already. The first we consider a large usability issue. Without a fix for it a good amount of the testing that we have put forward here will not be doable. At this time please report any problems that you may encounter to me. If you need any help, please feel free to contact me any time. Thank you, Jeff Moran Acrobat Quality Engineer Adobe Systems, Inc 206.675.7981
Liz, this looked like it was working with the cookbook example. We tested by jumping to the last page of the PDF right after it loaded then going backwards and forwards and reloading. There was a little "hickup", but it went to the last page almost instantly. In any case, since the numbers look right, it's just the placement of the "dash", I'm sure we can resolve this quickly.
I can explain why your test worked by serendipity: 1 - When you have disabled background downloading, after page 1 of a PDF file downloads, Acrobat does a little bogus read just to kill that initial download. Our bogus read is for two ranges: 0-1,2-3. 2 - mozilla turned this into the following range request: 01-, 23- The semantics of a byte range with a "<start of range> - " but no ending range is "download from the start of the range to the end of the file. With that range request, therefore, mozilla asked for the rest of the file! 3 - So when you asked to go to some page later in the file, it was already there.
I have some easy test cases for you now for this bug on an extra server. For these tests, it doesn't matter if background downloading is enabled or disabled. In the first test, the PDF file opens and starts downloading. Then Acrobat realizes that the document is supposed to open to page 5, so we do a read request for the data for page 5 and then flip to it. http://access.adobe.com/browser/netscape/readtesting/gotopage5.pdf If your fix is working correctly, the file will open with page 5. If it is not working, then the screen will be blank. The next three examples have the PDF files opening with "continuous facing page" mode -- like a book. When the PDF file opens, the first page will be downloaded and painted, and then we will issue a read request for more pages, which will then be downloaded and painted also. If it is working correctly, you will end up with at more than 2 pages displayed at a time on the screen (compare with Nav 4.X to see the exact number of pages -- which depends on how big your browser window is). http://access.adobe.com/browser/netscape/readtesting/starr.pdf http://access.adobe.com/browser/netscape/readtesting/garden.pdf http://access.adobe.com/browser/netscape/readtesting/Whamba.pdf If your fix is not working, the pages won't paint and acrobat will hang for five minutes waiting for data that it never gets.
Liz, thanks for the excellent testcases!
From the last comment, I think it could be something in nsPluginStreamInfo::MakeByteRangeString(nsByteRange* aRangeList, char** rangeRequest, PRInt32 *numRequests) (in nsPluginHostImpl). I need to set some breakpoints or printf to see what this method is returning. This bug is second in priority to bug 82445 and bug 82415.
Assignee: dougt → peterlubczynski
Status: REOPENED → NEW
Looks like it was just a typo from the copy/paste. PDT: Do I need approvals to fix Doug's typo? Strange though...seems to work in the debugger (as Liz describes) but fails debug on Win32. Now building optimized Win32 and debug on Mac to try this out.
Status: NEW → ASSIGNED
We're not out of the woods yet. That patch fixes the format of the request, but there are still other problems. For example, I'm getting a crash in _requestread as a result of invalid pointer in pstream->ndata. I'm gussing that we're deleting it but not nulling out the pointer. Rebuilding Debug so I can debug.
> PDT: Do I need approvals to fix Doug's typo? Not sure what you mean. It doesn't look like the patch has had any reviews yet, so you wouldn't be asking for re-review. I think the thing to do is for you to make a new diff which includes dougt's patch and your correction, and get r and sr on the combined diff.
Blocks: 76892
See the more inclusive, last patch in: http://bugzilla.mozilla.org/show_bug.cgi?id=82415 It should fix this bug as well as I have this working in my trees.
Depends on: 82415
Whiteboard: Patch ready - need review. → wanted for 0.9.1 Patch ready - need review.
This should now be working. Marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → FIXED
HEY, HEY, HEY! I have only had about a half hour so far to take this for a spin, but it is looking REALLY good! I will spend all Friday morning testing the corner cases.
The Browser QA god has been beating on this fix for me. It is looking very, very, very good. I did, however, just report one crash associated with byte range requests: 84332.
good. how reproducible is the crash? Can we ship 0.9.1 without fixing this bug?
Crash is completely reproduceable. I think someone needs to look at it to see what is causing the crash. (I'd do it but I am testing the DDE fix next.)
marking this VERIF/ FIXED (to get off radar) since new bug has been filed for the other issue. Thnx for testing this!
Status: RESOLVED → VERIFIED
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: