Closed Bug 53122 Opened 24 years ago Closed 24 years ago

OnFileAvailable is no longer called

Categories

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

defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: shrir, Assigned: dp)

References

()

Details

(Keywords: regression, Whiteboard: nsbeta3+)

Build:2000091808m18 windows Using today's windows build, try to load the above url Observe that the throbber shows the download progressing but nothing happens. Acrobat reader is not launched as a plugin and a blank page loads. Mail details from Sean : I did a pull this morning at 11am Mtn View time and am running into problems with plugins. I was wondering if you are seeing any of these. Basically OnFileAvailable is no longer called (it was working last week).
nsbeta3 nomination.
Keywords: nsbeta3
Summary: OnFileAvailable is no longer called → OnFileAvailable is no longer called
Adding Ed Burns
Av, I talked to Sean on the phone about this, and he mentioned to me that he was observing this before and after applying the patch for 52963. I have a tree from Wednesday night at 11pm, but it has all my fixes recently put into the tree, including 52963. I find that I CAN use mozilla to view PDF files using nppdf32.dll with this tree. Thus, at this point, I don't think it's any of my changes. I'm going to pull a new tree and do a clobber_all build and retest.
nsPluginStreamListenerPeer::OnStopRequest() has the following line: rv = channel->GetLocalFile(getter_AddRefs(localFile)); This gets the locally cached file that was GetURL'd, which is then passed to OnFileAvailable(). If the GetLocalFile() call fails, OnFileAvailable() does not get called. GetLocalFile() makes its way to nsHTTPChannel::GetFile() in http://lxr.mozilla.org/seamonkey/source/netwerk/protocol/http/src/nsHTTPChannel. cpp. GetFile() is failing because mCacheEntry is null (the rawptr in the mCacheEntry nscomptr is null). Possibly due to: http://bonsai.mozilla.org/cvsview2.cgi? diff_mode=context&whitespace_mode=show&file=nsHTTPChannel.cpp&root=/cvsroot&subd ir=mozilla/netwerk/protocol/http/src&command=DIFF_FRAMESET&rev1=1.230&rev2=1.231
Yep - OnStopRequest() is now getting called after mCacheEntry has been released - see rev 1.231 of nsHTTPChannel.cpp.
is this NT only? Or is bug 53056 a dup? ("Helper application for streaming mp3's fails to launch")
It's not NT only. I don't know how helper apps get notified that the file is ready - if there is a stream listener that waits for an OnStopBinding call and it tries to get the local cache file using the channel's mCacheEntry member, then it's a dupe ;)
OS: Windows NT → All
Hardware: PC → All
adding Suresh to cc list. Suresh, the change you made last week to nsHTTPChannel.cpp (mCacheEntry is now released before calling a listener's OnStopBinding) is causing a problem with plugins that use StreamAsFile. Can you take a look at bug 53122 and make a recommendation?
I'm marking this nsbeta3+ P2 and adding regression. DP, it sounds like you introduce a regression to plugins. Can you please consult with av or sean to resolve this.
Assignee: av → dp
Keywords: regression
Priority: P3 → P2
Whiteboard: nsbeta3+
Abolutely. Right away.
Status: NEW → ASSIGNED
To prevent cache entries from being open to too long, we tried to release every reference to the cache entry as soon as possible. And this change of releasing the nsHTTPChannel::mCacheEntry before stop notification was sent out was done. I guess we can go two ways: 1. Release the mCacheEntry in the channel after calling onStop. 2. Have listeners hold their own reference to the mCacheEntry. That is what other listeners were doing. I think we should go with 1 as the reason for not doing that was to release the entry early. But if every stoplistener is holding a reference anyway, it doesn't matter if the channel releases it reference before or after the stop request notification. I will add a patch very soon here. Ccing Neeti and Gagan, the owners of the code.
Hello: Eric Krock called several folks at Adobe today to get us to be more committed to verifying Acrobat functionality with mozilla. Unfortunately, this bug is blocking even the most basic testing. There are a lot of features that I am worried are still quite broken: something we call "background downloading" == "navigator keeps the initial connection to the file pouring until we do a read request". Also, byte range requests ... which haven't worked at all in any of the releases I have tested. Acrobat forms posting hasn't worked either. I can't test any of these things until we are past this basic bug.
adding to cc list
patch : apply this to netwerk/protocol/src/nsHTTPChannel.cpp Can some one confirm that this gets fixed. Neeti/Gagan, can you review approve the fix. I can check this in tonight. *************** *** 1860,1869 **** } } - // Release the cache entry as soon as we are done. This helps as it can - // flush any cache records and do maintenance. - mCacheEntry = nsnull; - // // Call the consumer OnStopRequest(...) to end the request... //---------------------------------------------------------------- --- 1860,1865 ---- *************** *** 1878,1883 **** --- 1874,1883 ---- this, rv)); } } + + // Release the cache entry as soon as we are done. This helps as it can + // flush any cache records and do maintenance. + mCacheEntry = nsnull; // // After the consumer has been notified, remove the channel from its
tested w/ acrobat and an xpcom plugin. Looks good. thanks!
Upping priority as we cant ship without this. I have reviews done. Will do in now...11:45pdt Thanks Sean for the quick turn around in testing the patch.
Priority: P2 → P1
Checked in. Thanks for all the help and locating this. Sorry about the regression.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
vrfy
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.