Closed
Bug 53122
Opened 24 years ago
Closed 24 years ago
OnFileAvailable is no longer called
Categories
(Core Graveyard :: Plug-ins, defect, P1)
Core Graveyard
Plug-ins
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).
Reporter | ||
Comment 1•24 years ago
|
||
nsbeta3 nomination.
Keywords: nsbeta3
Summary: OnFileAvailable is no longer called → OnFileAvailable is no longer called
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.
Comment 4•24 years ago
|
||
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
Comment 5•24 years ago
|
||
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")
Comment 7•24 years ago
|
||
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
Comment 8•24 years ago
|
||
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?
Comment 9•24 years ago
|
||
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 | ||
Comment 11•24 years ago
|
||
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.
Comment 12•24 years ago
|
||
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.
Comment 13•24 years ago
|
||
adding to cc list
Assignee | ||
Comment 14•24 years ago
|
||
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
Comment 15•24 years ago
|
||
tested w/ acrobat and an xpcom plugin.
Looks good.
thanks!
Assignee | ||
Comment 16•24 years ago
|
||
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
Assignee | ||
Comment 17•24 years ago
|
||
Checked in. Thanks for all the help and locating this. Sorry about the regression.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•