96648 - N620, M095 crash with plugins [@ nsPluginInstanceOwner::ProcessEvent]

Status: VERIFIED WORKSFORME

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

Description

[greer]

This one may be a dupe of bug 84681. But that one comments on using the code in 
nsObjectFrame.cpp nsPluginInstanceOwner to patch nsPluginInstanceOwner in 
nsPluginViewer.cpp. It was also verified fixed on the branch prior to RTM. 

This one is crashing in nsObjectFrame.cpp. It does appear to be fixed on the 
Trunk.

Dupe it or mark it fixed if appropriate. I'll include the stack and comments 
for good measure.

         nsPluginInstanceOwner::ProcessEvent()  [nsObjectFrame.cpp  line 2908]
         nsPluginInstanceOwner::DispatchMouseToPlugin() [nsObjectFrame.cpp  
line 2875]
         nsPluginInstanceOwner::MouseDblClick() [nsObjectFrame.cpp  line 2851]
         nsEventListenerManager::HandleEvent()  [nsEventListenerManager.cpp  
line 1265]
         nsGenericElement::HandleDOMEvent()     [nsGenericElement.cpp  line 
1673]
 
        Source File : nsObjectFrame.cpp line : 2908
     (34407607) URL: www.ultrashock.com
     (34407607) Comments: interacting with Flash Movie
     (34406962) URL: http://www.purepath.co.uk
     (34406962) Comments: Following site instructions to zoom in on a product
block diagram illustration (Option-click)
     (34398726) Comments: Viewing a Quicktime VR
     (34360063) URL: http://fr.barbie.com/
     (34360063) Comments: Trying to dress the mannequin
     (34306511) URL: www/Corel.com
     (34306511) Comments: Chatting on IM
     (34302470) URL: http://abadan.8k.com
     (34302470) Comments: scrolling with microsoft intellimouse
     (34285818) URL: http://www.elvis.com
     (34285818) Comments: Was viewing an IPIX virtual tour of Graceland at
elvis.com while another browser window was open to espn.com's baseball
scoreboard.
     (34270483) Comments: waiting for a quicktime movie to download

Comment 1

[greer]

Adding crash and topcrash keywords to make talkback happy.

Comment 2

[Peter Lubczynski]

Petersen, can you see if you can reproduce this? Thanks!

Comment 3

[shrirang khanzode]

i could repro a crash on going to fr.barbie.com and clicking on the yellow 
flower vase in the flash movie..dunno if that's the same crash.

Comment 4

[shrirang khanzode]

uh oh..i meant

1 click on the the printer 
2 then immediately click on the 'orange' vase

you should crash

Comment 5

[Peter Lubczynski]

Attachment: possible null-pointer check patch

Comment 6

[Peter Lubczynski]

I was unable to reproduce this on Mac, but I did notice that at one point we
could crash if mWidget is null. There was a check for this before, but it was
removed. My patch adds this back in but I'm not sure this will fix this bug if
this bug is still happeneing.

Comment 7

[Peter Lubczynski]

Lowering to P2 because it is hard to reproduce.

Comment 8

[greer]

There was a single comment in the M094 data today, it might be a viable place to
try to repro this one:

(35677588) Comments: Nokia Music Player pop up window...

The user didn't list a URL. But this one is likely:
http://www.nokia.com/phones/3330/musicplayer.html and click on one of the links
in the body. I tried to crash it but was unsuccessful. Maybe someone else can.

Comment 9

[lchiang]

peterl - since you have a null pointer check patch for this bug, I'm wondering
if we should take your patch for the branch.

Perhas you can check into the trunk after r and sr and then we can see if the
talkback incidents for this lessens on the trunk.

Comment 10

[Peter Lubczynski]

I still need to test this to make sure it doesn't cause regressions in edge cases.

Comment 11

[Peter Lubczynski]

Attachment: updated patch

Comment 12

[Peter Lubczynski]

Fixed up the patch. We should not be in ProcessEvent if we don't have a native
window to draw too!

I'm not sure this will fix this crash, if it's even happening still, but it
shouldn't hurt. It's a protective measure, but I don't see how we can even be
inside ProcessEvent if window is null.

Andrei, can you review?

Comment 13

[av (gone)]

Comment on attachment 51583 [details] [diff] [review]
updated patch

r=av

Comment 14

[Peter Lubczynski]

Comment on attachment 51583 [details] [diff] [review]
updated patch

sr=attinasi

marc super reviewed this, however, both he and I now don't think this patch will fix this top crash as talkback indicates a problem in nsObjectFrame.cpp and this patch is for nsPluginViewer.cpp

Comment 15

[Peter Lubczynski]

I tried all the testcases in this bug and I can not reproduce, adding minus. I
need very detailed steps on how to reproduce because this is likely an isolated
event problem.

Comment 16

[shrirang khanzode]

" Additional Comments From shrirang khanzode 2001-08-23 10:51 "

..is when I was able to repro this crash. But on later builds, I could not, I 
will try on latest branch build and see if I can get one.

Comment 17

[greer]

M094 data shows ten incidents under this signature. No meaningful comments:
(35987260) MacOS version 9.1 - trying to load a Shockwave movie
(36385726) MacOS version 9.0.4 - flash animation

Updating summary.

shrirang, any luck with a repro for this one?

Comment 18

[shrirang khanzode]

Forgot to update this bug...I still cannot reproduce. Will try again :(

Comment 19

[Peter Lubczynski]

Hmm...the second null pointer check patch in this bug is for
"pluginInstanceOwner" (notice no ns and lowercase) but this bug is about the
other crash, in nsObjectFrame.cpp. However, looking at today's NS62 talback
rankings, there is a NEW crash in this method (ranked 21st today!) Since this
patch has a review and super review, I checked it into the trunk.

I'm leaving this bug open to deal with the crash in nsObjectFrame.cpp. On that
trace, latest incident at 2001-10-11 11:56:4 does not indicate any comments to
help reproduce but goes through another kind of mouse event:

nsPluginInstanceOwner::ProcessEvent() [nsObjectFrame.cpp, line 2994]
nsPluginInstanceOwner::DispatchMouseToPlugin() [nsObjectFrame.cpp, line 2964]
nsPluginInstanceOwner::MouseOut() [nsObjectFrame.cpp, line 2947]
nsEventListenerManager::HandleEvent() [nsEventListenerManager.cpp, line 1306]
nsGenericElement::HandleDOMEvent() [nsGenericElement.cpp, line 1686] 

Comment 20

[greer]

Attachment: Attaching today's N620 comments showing platform and URL infor.

Comment 21

[greer]

The above comments show only one incident on M095 and no Trunk crashes in the 
last ten days of data. However, the comments provide a number of sites for use 
in testing/repro with the N620 release.

Comment 22

[Peter Lubczynski]

I now see this on my Carbon Trunk build, but ONLY in a debug build by simply
clicking to play a Quicktime movie. It works fine in a commercial build though,
weird. Good thing is that I break in CW7 and now see what's going on:
http://lxr.mozilla.org/mozilla/source/layout/html/base/src/nsObjectFrame.cpp#3195

I think the cast to an EventRecord here may not be correct, however CW7 hangs
when I try to expand the nsGUIEvent and doesn't break in that method when I set
a breakpoint earlier.

Greer: does talkback see this crash anymore in OS9? Maybe it's just OSX now?

Comment 23

[greer]

Peter, somehow I missed your comment on 12-13. Sorry about that. I just checked
the  Talkback data and don't find any incidents on M097 or the Trunk With the
nsPluginInstanceOwner::ProcessEvent signature, nor any crashes in nsObjectFrame.cpp

Last release having this signature was N621.

Marking WFM.

Comment 24

[shrirang khanzode]

wonderful, VERIFIED