57725 - Webclient (PR3) core dumps when trying to Navigate links on Solaris

Status: VERIFIED FIXED

(Core Graveyard :: Java APIs to WebShell, defect, P3)

Description

[Ashu Kulkarni]

When Webclient is run on Solaris and one tries to navigate links, Mozilla
seg-faults in nsWindow.cpp due to null-ptr dereference.

This is because in Webclient, the top level Window wrapping the WebBrowser
object is not a gtk window, but rather, a motif window (Java native peer). In
mozilla/widget/src/gtk/nsWindow.cpp::SetFocus, when the code tries to get a
nsWindow from the GtkWidget, a null is returned. When this is dereferenced,
without checking whether nsWindow is null, a seg fault occurs.

The fix for this, is to check if the nsWindow returned by the
gtk_object_get_data(GTK_OBJECT(top_mozarea), "nsWindow");
call is null.

The cvs diff -u for the fix is as follows -

Index: nsWindow.cpp
===================================================================
RCS file: /cvsroot/mozilla/widget/src/gtk/nsWindow.cpp,v
retrieving revision 1.305
diff -u -r1.305 nsWindow.cpp
--- nsWindow.cpp        2000/09/22 06:48:56     1.305
+++ nsWindow.cpp        2000/10/23 16:30:46
@@ -1085,15 +1085,17 @@
     if (!GTK_WIDGET_HAS_FOCUS(top_mozarea))
     {
       nsWindow *mozAreaWindow = (nsWindow
*)gtk_object_get_data(GTK_OBJECT(top_mozarea), "nsWindow");
-      mozAreaWindow->mBlockMozAreaFocusIn = PR_TRUE;
+      if (mozAreaWindow) {
+        mozAreaWindow->mBlockMozAreaFocusIn = PR_TRUE;
 #ifdef DEBUG_FOCUS
-      printf("mozarea grabbing focus!\n");
+        printf("mozarea grabbing focus!\n");
 #endif
-      gtk_widget_grab_focus(top_mozarea);
-      // this will show the window if it's minimized and bring it to
-      // the front of the stacking order.
-      GetAttention();
-      mozAreaWindow->mBlockMozAreaFocusIn = PR_FALSE;
+        gtk_widget_grab_focus(top_mozarea);
+        // this will show the window if it's minimized and bring it to
+        // the front of the stacking order.
+        GetAttention();
+        mozAreaWindow->mBlockMozAreaFocusIn = PR_FALSE;
+      }
     }
   }


Since in the normal browser operation, the GtkWidget is an actual gtk window,
the call to gtk_object_get_data never returns a null. 

This fix is required to get Webclient working with Mozilla PR3 codebase.

_Ashu

Comment 1

[Ashu Kulkarni]

I filed, I accept.

_Ashu

Comment 2

[edburns]

Looks good to me r=edburns.  Let's get r=blizzard and sr= someone and get this
into the trunk.

Comment 3

[Christopher Blizzard (:blizzard)]

Are you guys creating the mozarea object and passing it in as the parent as part
of the Create() call?  I do something similar in the embedding widget and I
never get crashes where you do.

Comment 4

[Ashu Kulkarni]

Chris,

We are creating the MozArea and passing it to the Create call just as you are in
yout gtkembed example. Check out
webclient/src_moz/NativeEventThread.cpp:line#608
in lxr.

The reason our MozArea is diff from yours is that our top level window is a
native motif window (what Java uses as its native peers) while yours is a gtk
window.

_Ashu

Comment 5

[Christopher Blizzard (:blizzard)]

Acutally, in the gtk embedding example I pass in a standard GtkContainer, not a
mozarea as the parent of the top mozilla window.  The mozarea object is created
and managed by mozilla.  Have a look at:

http://lxr.mozilla.org/seamonkey/source/embedding/browser/gtk/src/gtkmozembed.cpp#1362

To be honest, I'm not sure what the effect of having a mozarea object that's not
managed by mozilla will be.  I would expect other bugs to crop up.  Would you be
willing to pass in a GtkContainer instead of creating the mozarea yourself?

Comment 6

[Ashu Kulkarni]

Take a look at
http://lxr.mozilla.org/mozilla/source/java/webclient/src_moz/motif/MotifBrowserControlCanvas.cpp#77
to see how I create the GtkWidget and mozArea. What is returned in this function
is passed to the BaseWindow->Create call in
http://lxr.mozilla.org/mozilla/source/java/webclient/src_moz/NativeEventThread.cpp#600

I tried passing the top level GtkContainer, but nothing was being displayed, not
even the Java Window.

Only if I create the MozArea myself and pass it in like I do here, does the
embedded Browser come up.

Comment 7

[timeless]

Blizzard: please read and comment.

Comment 8

[Christopher Blizzard (:blizzard)]

You really want to create something like a GtkBin class and pass that into the
CreateNative() call.  If you look at the CreateNative() all in nsWindow.cpp

http://lxr.mozilla.org/seamonkey/source/widget/src/gtk/nsWindow.cpp#1612

you can see that it looks to see if the parent is a GtkContainerClass or child
thereof.  If it is, then it will create the mozArea which is really what you
want.  Just make sure that the widget that you pass in is realized first.

Comment 9

[Ashu Kulkarni]

I create a mozArea, and pass it's SuperWin to the Create call. In
nsWindow::CreateNative, it checks to see if the widget is a SuperWin and handles
it correctly.

Look at http://lxr.mozilla.org/seamonkey/source/widget/src/gtk/nsWindow.cpp#1718

From looking at nsWindow.cpp, this seems to be a valid approach. If I just
create a GtkContainer and pass it to the Create call, nothing gets drawn and it
does not complain about the widget not being realised first (I suppose Gtk would
complain if the Widget hadn't been realised).

Comment 10

[Christopher Blizzard (:blizzard)]

The code supports that because that's how the native child windows are created.
 However, the code doesn't deal with the case where the mozarea widget that is
passed was not created by the mozilla code itself, like you are doing.

Are you passing in a GtkContainer or a GtkBin?  The GtkContainer is a base class
that doesn't have a lot of functionality on its own.  The GtkBin class is a
container that holds exactly one child which is what you want.

Comment 11

[Ashu Kulkarni]

Attachment: cvs diff -u of MotifBrowserControlCanvas.cpp and NativeEventThread.cpp

Comment 12

[Ashu Kulkarni]

Creating a GtkBin object and passing it to BaseWindow->Init fixed the problem.
The patch for Webclient is attached. With this fix no changes are needed in the
mozilla code.

thanks for your help in this Chris!

_Ashu

Comment 13

[Ashu Kulkarni]

Ed,

Pls verify fix so I can close this Bug.

_Ashu

Comment 14

[Ashu Kulkarni]

Attachment: tar of modified files. untar in java/webclient

Comment 15

[Ashu Kulkarni]

Patch modifies
src_moz/NativeEventThread.cpp
src_moz/motif/MotifBrowserControlCanvas.cpp

_Ashu

Comment 16

[edburns]

The fix is not checked in, correct?

Comment 17

[Ashu Kulkarni]

it's not checked in. I'm waiting for r=a=edburns

_Ashu

Comment 18

[edburns]

This looks good.  r=a=edburns.  Just remove the printfs.

Comment 19

[Ashu Kulkarni]

Fix checked in. Closing this Bug.

Comment 20

[Alexei V. Mokeev]

Work now. Mark VERIFIED