Browser stops responding when url is loaded and back button is pressed

VERIFIED FIXED

Status

P2
critical
VERIFIED FIXED
19 years ago
9 years ago

People

(Reporter: shrir, Assigned: edburns)

Tracking

({crash})

Trunk
x86
Windows NT
crash
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3+][PDTP2], URL)

Attachments

(14 attachments)

11.84 KB, patch
Details | Diff | Splinter Review
9.42 KB, patch
Details | Diff | Splinter Review
39.41 KB, application/octet-stream
Details
11.84 KB, patch
Details | Diff | Splinter Review
9.42 KB, patch
Details | Diff | Splinter Review
39.43 KB, application/octet-stream
Details
3.08 KB, patch
Details | Diff | Splinter Review
2.37 KB, patch
Details | Diff | Splinter Review
39.97 KB, application/octet-stream
Details
3.09 KB, patch
Details | Diff | Splinter Review
2.38 KB, patch
Details | Diff | Splinter Review
39.95 KB, application/octet-stream
Details
3.92 KB, patch
Details | Diff | Splinter Review
43.92 KB, application/octet-stream
Details
(Reporter)

Description

19 years ago
Build: 2000082804m18 windows

Go to the above url
After the page loads, press the BACK button
Observe that the url bar shows the previous url from the history but the page 
does not load and the browser goes in a limbo. Menus can be clicked but nothing 
functions.
Browser needs a  restart to be working again.
(Reporter)

Comment 1

19 years ago
nominating for nsbeta3 since this is a majorproblem and seen on lot of applet 
pages.
Keywords: nsbeta3
(Assignee)

Comment 2

19 years ago
reassign to edburns.
Assignee: drapeau → edburns
(Assignee)

Comment 3

19 years ago
Shirang, can you please post the testcase as an attachment so non-netscape 
employees can see it?

thanks,

Ed
Status: NEW → ASSIGNED
(Reporter)

Comment 4

19 years ago
oops..sorry about that. Updated the url.
(Assignee)

Updated

19 years ago
Blocks: 12153
(Assignee)

Updated

19 years ago
Blocks: 49247
(Assignee)

Comment 5

19 years ago
*** Bug 49438 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 6

19 years ago
*** Bug 21757 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 7

19 years ago
*** Bug 32129 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 11

19 years ago
Posted this to staff@mozilla.org

I know this isn't the place to solicit code review, but this bug fix is a 
substantial change in strategy for plugins.

Summary: 

Looking for approval to check in changes to the plugin lifecycle...make it be 
the 4.x plugin lifecycle.

Detail:

This fix was requested by Stanley Ho of the Sun Java Plugin Team.
Stanley floated a proposal some time ago on staff@mozilla.org,
n.p.m.{oji,java,plugins} about changing the mozilla plugin lifecycle to
be back to the ns4.x plugin lifecycle in the following respect.

Stanley's proposal:

When you visit a page with a mozilla (not 4.x) plugin:

  1. the plugin is loaded

  2. nsIPluginInstance::Initialize() is called

  3. nsIPluginInstance::Start() is called

When you leave the page with a running mozilla (not 4.x) plugin:

  1. nsIPluginInstance::Stop() is called

  2. nsIPluginInstance::Destroy() is called

  3. nsIPluginInstance::SetWindow(nsnull) is called

The bug fix for 50547 implements the above behavior.  Here's what I did:

  Remove method nsPluginHostImpl::FindStoppedPluginForURL().

  Remove method nsPluginHostImpl::AddInstanceToActiveList().

  Remove method nsIPluginHost::StopPluginInstance().

  Remove ivar nsActivePluginList mActivePluginList.

  Remove class nsActivePlugin.

  Remove class nsActivePluginList.

  Remove macro MAX_NUMBER_OF_STOPPED_PLUGINS.

Files in this patch:

M modules/plugin/nglsrc/nsIPluginHost.h
M modules/plugin/nglsrc/nsPluginHostImpl.cpp
M modules/plugin/nglsrc/nsPluginHostImpl.h
M modules/plugin/nglsrc/nsPluginViewer.cpp
M layout/html/base/src/nsObjectFrame.cpp

Please visit the bug to see the patches:

http://bugzilla.mozilla.org/show_bug.cgi?id=50547
(Assignee)

Comment 12

19 years ago
Pending approval, looking at ETA 11 Sept 00.

Comment 16

19 years ago
1. A background question.  How does getting rid of the plugin cache affect the 
applets bug?  Specifically, what is currently cached: applet instances, the 
java plugin or both?

2. nsPluginViewer.cpp - should remove these lines also:
     nsCOMPtr<nsIPluginHost> host;
     host = do_GetService(kCPluginManagerCID);

3. As best as I can tell, calling SetWindow() after a plugin has been     
Destroy()'d is not 4x behavior and doesn't seem like a safe thing to start 
doing.

(Assignee)

Updated

19 years ago
Blocks: 15179
(Assignee)

Comment 17

19 years ago
Well, getting rid of the plugins cache is a necessary effect of causing the 
browser to send Destroy() to the plugin on page switch.  Sending Destroy() on 
page switch is what this is about, and we can't keep an entry in the "running 
plugins" list for plugins that have been destroyed.

Comment 18

19 years ago
So the problem is only that the java plugin is cached?

If so, an alternative is to add another java special case like this:
    (PL_strcasecmp(aMimeType, "application/x-java-vm") != 0 && 
     PL_strcasecmp(aMimeType, "application/x-java-applet") != 0))

before the call to AddInstanceToActiveList() in 
nsPluginHostImpl::SetUpPluginInstance().

Then in the java plugin Stop() method, call Destroy() directly (since it won't 
come from the nsActivePlugin dtor).

Comment 19

19 years ago
Or even better, add a method to nsIPluginInstance like:
PR_BOOL OkToCache();

And call it inside of AddInstanceToActiveList().

So that plugins instances can govern their own destiny.

Comment 20

19 years ago
Updating status: fixing this will resolve critical crash issue for Java.
Severity: major → critical
Keywords: crash
Priority: P3 → P1
Whiteboard: [nsbeta3+]
(Assignee)

Comment 21

19 years ago
Adding CCs.

I'm about to attach the fix for this bug discussed in the conference call today.

Here's my cvs log message for this checkin.

Summary:

Looking for approval to check in changes that allow plugins to indicate:

1. Whether or not they want to allow the browser to cache their
instance.  Default is yes, do allow the browser to cache their instance.

2. If they answer no to 1, that is, no the plugin does not want the
browser to cache their instance, do you want the shutdown calls to be:

a. 

          inst->SetWindow(nsnull);
          inst->Stop();
          inst->Destroy();


b.

          inst->Stop();
          inst->Destroy();
          inst->SetWindow(nsnull);

a. is the default.

Please visit the bug to see the patches:

http://bugzilla.mozilla.org/show_bug.cgi?id=50547

Detail:

This fix was requested by Stanley Ho of the Sun Java Plugin Team.  A
conference call between Eric Krock, Andrei Volkov, Sun, Adobe and other
plugin vendors was used to agree on the above solution.

M modules/plugin/public/nsplugindefs.h
M modules/plugin/nglsrc/nsPluginHostImpl.cpp
M layout/html/base/src/nsObjectFrame.cpp

(Assignee)

Comment 25

19 years ago
PLEASE note that a new build of the java plugin that honors these changes is 
required to observe the fix for this bug.

Comment 26

19 years ago
Ed, should not it be 
   if (!doCache) 
in place of your 
   if (doCache)
in nsObjectFrame.cpp?
(Assignee)

Comment 27

19 years ago
Sean, yes, definately PR_TRUE.  

Sean, yes, should be !doCache.  I'm going to repost diffs and tar.gz.

(Assignee)

Comment 30

19 years ago
(Assignee)

Comment 31

19 years ago
Yes, Sean already caught that.  I've fixed it in the fourth iteration patches.

Comment 32

19 years ago
Looks like the solution everyone agreed to.

I know we didn't discuss this explicitly (because noone thought of it), but I 
think we also need to modify nsPluginViewer.cpp:
http://lxr.mozilla.org/seamonkey/source/modules/plugin/nglsrc/nsPluginViewer.cpp
#754

The pluginInstanceOwner assumes that a plugin instance is cached.  No error is 
returned from StopPluginInstance if the plugin is not found in the cache and 
destroy will never be called on it (if the plugin instance opted out of the 
cache).

a=brendan@mozilla.org if sean's comment in the bug is an r= (is it?  What bug is 
on file for fixing nsPluginViewer.cpp?).

/be

Comment 34

19 years ago
There won't be anything wrong with nsPluginViewer.cpp until the attached patch 
goes in.  It needs to be aware of the plugin lifecycle change this patch 
addresses for nsObjectFrame.cpp so that plugins can be destroyed properly (if 
they opted out of the cache - it currently works correctly if plugin instances 
are in fact cached).  

nsPluginViewer.cpp is used by full-page plugin instantiations.  Full-page 
instantiations occur when you click on a file whose mimetype is handled by an 
installed plugin (no html file involved).  The code touched in nsObjectFrame is 
not run in the case of full-page plugins.


(Assignee)

Comment 35

19 years ago
Sean, I see, I'll make the change in nsPluginViewer as well.  I'll re-post the 
diffs.
(Assignee)

Comment 36

19 years ago
Sean, nsPluginViewer's Destroy is a no-op.
Are you on IRC or AIM or something?

Comment 37

19 years ago
Sorry - was out to lunch.

Regarding the Destroy() impl, correct.  But the line of importance is:
      host->StopPluginInstance(mInstance);
in the pluginInstanceOwner destructor.

That call relies on the instance being in the cache (in which case Destroy() is 
called when the plugin is removed from the cache).  If the plugin is not in the 
cache, then we need to call Destroy() on the instance after the Stop() instead 
of calling StopPluginInstance().  

(Assignee)

Comment 38

19 years ago
For the record, make it so the beginning of pluginInstanceOwner :: 
~pluginInstanceOwner() does the same thing as nsObjectFrame::Destroy().

Comment 41

19 years ago
The only thing about the nsPluginViewer.cpp change is that this block could be 
an else block after the if (!doCache):
  nsCOMPtr<nsIPluginHost> host;
  host = do_GetService(kCPluginManagerCID);
  if(host)
    host->StopPluginInstance(mInstance);

As things are now, nothing bad happens.  The StopPluginInstance call will just 
fail.

r=sean
tested on WinNT with 4x adobe acrobat in full-page mode
tested with xpcom plugin with and without cache option in embed mode

Comment 42

19 years ago
*** Bug 48050 has been marked as a duplicate of this bug. ***

Updated

19 years ago
Blocks: 14712

Comment 43

19 years ago
PDT thinks this bug is a P2 because Java is only installed if you choose
"Complete". Most users will install "Typical" and thus won't see this bug.
Priority: P1 → P2
Whiteboard: [nsbeta3+] → [nsbeta3+][PDTP2]
(Assignee)

Comment 44

19 years ago
This one is fixed.  However, you'll need a version of the JavaPlugin more 
recent than 18 Sept to verify it.
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED
(Reporter)

Comment 45

19 years ago
Java plugin info :Java(TM) Plug-in: Version 1.3.0_01
                  Using JRE version 1.3.0_01-beta Java HotSpot(TM) Client VM
                  
Verified this is working fine on windows commercial br build 2000092908.Marking 
as such.
Status: RESOLVED → VERIFIED

Updated

9 years ago
Component: Java: OJI → Java: OJI
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.