506433 - reduce the number of GdkWindows used in nsIWidgets

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Karl Tomlinson (:karlt)]

Child nsIWidgets usually have two GdkWindows.  Toplevel nsIWidgets have four.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: part 1: replace mozDrawingarea with a single GdkWindow

This removes one GdkWindow from any nsIWidget.

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: part 2: use the window in MozContainers

MozContainers already have a GdkWindow, so let's not create another one.
This removes another GdkWindow from toplevel nsIWidgets (and other container nsIWidgets).

Comment 3

[Karl Tomlinson (:karlt)]

I'd better have a look at test_container too, though that doesn't seem to be part of any build.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 390625 [details] [diff] [review]
part 1: replace mozDrawingarea with a single GdkWindow

         else if (mContainer) {
             gtk_widget_hide(GTK_WIDGET(mContainer));
-            moz_drawingarea_set_visibility(mDrawingarea, FALSE);
-        }
-        if (mDrawingarea) {
-            moz_drawingarea_set_visibility(mDrawingarea, FALSE);
+            gdk_window_hide(mGdkWindow);
+        }
+        if (mGdkWindow) {
+            gdk_window_hide(mGdkWindow);

Shouldn't that be "else if (mGdkWindow)", matching the aAction true case?

Comment 5

[Karl Tomlinson (:karlt)]

(In reply to comment #4)
> Shouldn't that be "else if (mGdkWindow)", matching the aAction true case?

Yes, it should.  That gets fixed up in attachment 390626 [details] [diff] [review].

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: test_container changes

test_container didn't build even without attachment 390625 [details] [diff] [review].
These changes are enough that test_container can be built and run, but the child widgets are not really visible because moz_container_allocate_child doesn't really do anything.  (Our only child widgets are plugin sockets which set their own size.)

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Let's just remove test_container. It's not testing anything that isn't tested by all our other tests.

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: remove test_container

Comment 9

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/mozilla-central/rev/d977079ae093
http://hg.mozilla.org/mozilla-central/rev/46bcba3b3241
http://hg.mozilla.org/mozilla-central/rev/b6f8a4610472