If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Mozilla cannot open PDF document via proxy and It crash after close the tab when it loaded

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
Plug-ins
P3
normal
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: Leon Sha, Assigned: Leon Sha)

Tracking

({helpwanted})

Trunk
mozilla1.9alpha1
Sun
Linux
helpwanted
Points:
---
Bug Flags:
blocking1.8b3 -
blocking1.8b5 -
blocking1.8rc1 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: need more input, URL)

Attachments

(3 attachments, 1 obsolete attachment)

45.04 KB, application/octet-stream
Details
43.55 KB, application/octet-stream
Details
2.83 KB, patch
Darin Fisher
: review+
shaver
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

13 years ago
Launch Mozilla, browse to
http://mediacast.sun.com/share/leon/JDSR3GAG_8190919_d4.pdf. 
When you are using proxy, acroread appears to start (as mozilla plugin) but
nothing is displayed.
When you access the page directly or save this pdf to local file and open if,
the document successfully opens in mozilla's acroread plugin.

This bug only happened on linux and solaris, not on windows.
(Assignee)

Comment 1

13 years ago
The problem I found it here:
http://lxr.mozilla.org/seamonkey/source/modules/plugin/base/src/nsPluginHostImpl.cpp#6617

6603   mInstance->Stop();
6604   mInstance->Start();
..................................
6617         ((nsPluginNativeWindow*)window)->CallSetWindow(inst);

In ns4xPluginInstance::Stop, mozilla destoried mXtBin, but not window->ws_info.
So in ns4xPluginInstance::SetWindow mXtBin is null, but window->ws_info is not
null. This is not a correct situration.

This bug maybe related to bug 94895 and Bug 119494.
Thanks, Leon.  Giving to jst to track for 1.8b3.

/be
Assignee: nobody → jst
Flags: blocking1.8b3+
Priority: -- → P3
Target Milestone: --- → mozilla1.8beta3

Updated

12 years ago
QA Contact: plugins → asa
Whiteboard: need more input

Comment 3

12 years ago
Leon, can you please capture a HTTP log?  Instructions here:
http://www.mozilla.org/projects/netlib/http/http-debugging.html

Thanks!
(Assignee)

Comment 4

12 years ago
Created attachment 186295 [details]
http log with proxy enabled
(Assignee)

Comment 5

12 years ago
Created attachment 186296 [details]
http log with proxy disabled
(Assignee)

Comment 6

12 years ago
Before I get these logs, I apply the temp patch for bug 246560.

Comment 7

12 years ago
Darin, can you see anything obvious in these logs?
Assignee: jst → darin

Comment 8

12 years ago
darin will look at as soon as he gets over the hump on updater and other stuff.

Updated

12 years ago
Flags: blocking1.8b4+
Flags: blocking1.8b3-
Flags: blocking1.8b3+

Comment 9

12 years ago
This bug has more critical impact to users, steps to reproduce:
1. Start Mozilla.
2. Open a new tab and go to the following link:
http://java.sun.com/j2ee/1.4/docs/tutorial/doc/J2EETutorial.pdf
[BUG:the file can't display in browser]
3. Open another tab and go to http://www.sina.com.cn
[page load successfully]
4. Go back to the previous page and reload the page.
[PDF file displays correctly]
5. Close this tab either by right clicking the tab and selecting 'close tab' or
by clicking the X at the right upper corner.
[BUG:Mozilla crash; if you did close www.sina.com.cn tab instead of PDF tab, go
back to PDF tab, the PDF contents disappears.]

But this bug didn't happen to all the PDF files. It only happens to a small part
of PDF files. Unfortunately, we have not find which kind of PDF files cause this
bug. 
We tried 193 PDF files, only 5 had this problem. Test files can be found in the
following link:
http://acroeng.adobe.com/file_elements.html

Updated

12 years ago
Summary: cannot open PDF document via proxy using mozilla → Mozilla cannot open PDF document via proxy and It crash after close the tab when it loaded

Comment 10

12 years ago
Thanks for the extra details on reproducing this bug.  I plan to fix this for
Firefox 1.1 beta.
Status: NEW → ASSIGNED

Comment 11

12 years ago
WFM 

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050704
Firefox/1.0+ (stipe)

Windows XP SP2 + Acrobat Reader 7.02
(Assignee)

Comment 12

12 years ago
(In reply to comment #11)
> WFM 
> 
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050704
> Firefox/1.0+ (stipe)
> 
> Windows XP SP2 + Acrobat Reader 7.02

This bug only happened in linux/unix.

Updated

12 years ago
Flags: blocking1.8b4-
Flags: blocking1.8b4+
Flags: blocking-aviary1.5+

Updated

12 years ago
Flags: blocking-aviary1.5+

Updated

12 years ago
Target Milestone: mozilla1.8beta3 → mozilla1.8beta5

Updated

12 years ago
Flags: blocking1.8rc1?

Comment 13

12 years ago
Darin, do you think that a fix here will be isolated and low-risk?

Comment 14

12 years ago
I have no idea.  I haven't had time to investigate this bug yet.  I could
certainly use help.
Keywords: helpwanted

Comment 15

12 years ago
Do we have a stacktrace for the crash? I don't think this is going to make 1.5
without someone coming up with a very safe patch in the next few days. If that
happens, please request approval at the patch flag.
Flags: blocking1.8rc1? → blocking1.8rc1-

Updated

12 years ago
Target Milestone: mozilla1.8beta5 → mozilla1.9alpha
(Assignee)

Comment 16

12 years ago
Created attachment 200955 [details] [diff] [review]
Patch
Attachment #200955 - Flags: review?
(Assignee)

Updated

12 years ago
Attachment #200955 - Flags: review? → review?(darin)

Comment 17

12 years ago
Leon: can you please explain why this patch fixes the bug?
(Assignee)

Comment 18

12 years ago
(In reply to comment #17)
> Leon: can you please explain why this patch fixes the bug?
> 
http://lxr.mozilla.org/seamonkey/source/modules/plugin/base/src/nsPluginHostImpl.cpp#6662
After mInstance->Stop(), mXtBin has been destoried.
The fact is we does not really want to destory mXtbin here.
If mXtbin needs to be destoried, mInstance->Destory() should be called. So I just move the destory mXtbin part to mInstance->Destory().
I think Leon has a promising patch here.  jst should sr.

/be
Flags: blocking1.8rc1- → blocking1.8rc1?

Updated

12 years ago
Attachment #200955 - Flags: superreview?(jst)

Comment 20

12 years ago
I really don't understand this code at all.  What is mXtBin?  ns4xPluginInstance::Destroy says that destruction happens in ::Stop.  Was the original author just wrong about that?  Is there some better way to destroy mXtBin in Stop that does not cause this bug?  I really don't know this code well enough to say with any certainty that "yeah, this is the proper fix."
this patch is only for the second half of the summary, right?
(Assignee)

Comment 22

12 years ago
(In reply to comment #20)
> I really don't understand this code at all.  What is mXtBin? 
> ns4xPluginInstance::Destroy says that destruction happens in ::Stop.  Was the
> original author just wrong about that?  Is there some better way to destroy
> mXtBin in Stop that does not cause this bug?  I really don't know this code

Maybe we can do this. Just not use mInstance->Stop() here.
In nsPluginStreamListenerPeer::ServeStreamAsFile we just need to clean up the streams, so we give it a method to clean up the streams and do not use mInstance->Stop().

> well enough to say with any certainty that "yeah, this is the proper fix."
> 

(Assignee)

Comment 23

12 years ago
(In reply to comment #21)
> this patch is only for the second half of the summary, right?
> 
I don't quite understand you meaning.
(In reply to comment #23)
> I don't quite understand you meaning.

It doesn't fix "Mozilla cannot open PDF document via proxy", does it?

Comment 25

12 years ago
we should release note the save and open in standalone viewer workaround.
Flags: blocking1.8rc1? → blocking1.8rc1-
(Assignee)

Comment 26

12 years ago
Created attachment 207056 [details] [diff] [review]
Patch v2

This is a more reasonable patch. 
Since mXtBin is destroied, I create a new one here.
Attachment #200955 - Attachment is obsolete: true
Attachment #207056 - Flags: review?(darin)
Attachment #200955 - Flags: superreview?(jst)
Attachment #200955 - Flags: review?(darin)

Comment 27

12 years ago
Comment on attachment 207056 [details] [diff] [review]
Patch v2

>Index: nsPluginHostImpl.cpp

>+#if defined (MOZ_WIDGET_GTK) || defined (MOZ_WIDGET_GTK2)
>+      // Should call GetPluginPort() here.

Why aren't we calling GetPluginPort() then?

Aagin, I'm not very comfortable reviewing this code because I
don't know what it is doing.  Isn't there someone who knows
these APIs better?
Attachment #207056 - Flags: review?(darin) → review+
blizzard, caillon, maybe bryner?  shaver, any thoughts?

/be
(Assignee)

Comment 29

12 years ago
(In reply to comment #27)
> (From update of attachment 207056 [details] [diff] [review] [edit])
> >Index: nsPluginHostImpl.cpp
> 
> >+#if defined (MOZ_WIDGET_GTK) || defined (MOZ_WIDGET_GTK2)
> >+      // Should call GetPluginPort() here.
> 
> Why aren't we calling GetPluginPort() then?
> 

GetPluginPort() is not an interface of nsIPluginInstanceOwner.
It is just a function of nsPluginInstanceOwner.

> Aagin, I'm not very comfortable reviewing this code because I
> don't know what it is doing.  Isn't there someone who knows
> these APIs better?

I agree with you. It is better to have someone familiar with this module to review this. 
(Assignee)

Updated

12 years ago
Attachment #207056 - Flags: superreview?(shaver)

Updated

12 years ago
Assignee: darin → leon.sha
Status: ASSIGNED → NEW
Comment on attachment 207056 [details] [diff] [review]
Patch v2

sr=shaver
Attachment #207056 - Flags: superreview?(shaver) → superreview+
(Assignee)

Comment 31

12 years ago
Checking in base/src/ns4xPluginInstance.cpp;
/cvsroot/mozilla/modules/plugin/base/src/ns4xPluginInstance.cpp,v  <--  ns4xPluginInstance.cpp
new revision: 1.132; previous revision: 1.131
done
Checking in base/src/nsPluginHostImpl.cpp;
/cvsroot/mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp,v  <--  nsPluginHostImpl.cpp
new revision: 1.550; previous revision: 1.549
done
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.