1411018 - Polish mShell/mContainer initialization

Status: RESOLVED FIXED

(Core :: Widget: Gtk, enhancement, P1)

Description

[Martin Stránský [:stransky] (ni? me)]

Configure mShell/mContainer widget rendering for CSD.

Comment 1

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1411018 - Rename shellHasCSD to drawToContainer and mark our rendering widget by gtk_widget_set_app_paintable(),

Review commit: https://reviewboard.mozilla.org/r/192194/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/192194/

Comment 2

[Martin Stránský [:stransky] (ni? me)]

The question is if we even need this patch when CSD widget setup should be enabled automatically and we get the "csd" style from GdkWindow. I suspect the csd style is not always properly detected but I'll investigate it.

Comment 3

[Martin Stránský [:stransky] (ni? me)]

Moving to Bug 1399611 as the setup depends on CSD rendering implementation.

Comment 4

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8921201 [details]
Bug 1411018 - Rename shellHasCSD to drawToContainer and mark our rendering widget by gtk_widget_set_app_paintable(),

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192194/diff/1-2/

Comment 5

[Jan Horak [:jhorak]]

Comment on attachment 8921201 [details]
Bug 1411018 - Rename shellHasCSD to drawToContainer and mark our rendering widget by gtk_widget_set_app_paintable(),

https://reviewboard.mozilla.org/r/192194/#review199530

::: widget/gtk/nsWindow.cpp:3781
(Diff revision 2)
>          gtk_widget_realize(mShell);
>  
> -        // We can't draw directly to top-level window when client side
> -        // decorations are enabled. We use container with GdkWindow instead.
>          GtkStyleContext* style = gtk_widget_get_style_context(mShell);
> -        shellHasCSD = gtk_style_context_has_class(style, "csd");
> +        bool shellHasCSD = gtk_style_context_has_class(style, "csd");

If shellHasCSD can be avoided, please do so by using:

drawToContainer = gtk_style_context_has_class(style, "csd");

and put the following comment above.

::: widget/gtk/nsWindow.cpp:3786
(Diff revision 2)
> -            // Prevent GtkWindow from painting a background to flicker.
> -            gtk_widget_set_app_paintable(mShell, TRUE);
> +         *    Content is rendered to mShell window and we listen
> +         *    Gtk+ events on mShell.

nit: "Content is rendered to mShell window and we listen to the Gtk+ events on mShell."

::: widget/gtk/nsWindow.cpp:3790
(Diff revision 2)
> -            gtk_widget_set_has_window(container, FALSE);
> -            // Prevent GtkWindow from painting a background to flicker.
> -            gtk_widget_set_app_paintable(mShell, TRUE);
> +         * 1) We're running on Gtk+ without client side decorations.
> +         *    Content is rendered to mShell window and we listen
> +         *    Gtk+ events on mShell.
> +         * 2) We're running on Gtk+ > 3.20 and client side decorations
> +         *    are drawn by Gtk+ to mShell. Content is rendered to mContainer,
> +         *    we listen events on mContainer.

nit: "we listen to the events on mContainer"

::: widget/gtk/nsWindow.cpp:3800
(Diff revision 2)
> -        // Set up event widget
> -        eventWidget = shellHasCSD ? container : mShell;
> +#endif
> +        eventWidget = (drawToContainer) ? container : mShell;
> +
>          gtk_widget_add_events(eventWidget, kEvents);
>  
> +        // Prevent GtkWindow from painting a background to flicker.

You mean:
Prevent GtkWindow from painting the background to avoid flickering.
?

::: widget/gtk/nsWindow.cpp:3803
(Diff revision 2)
>          gtk_widget_add_events(eventWidget, kEvents);
>  
> +        // Prevent GtkWindow from painting a background to flicker.
> +        gtk_widget_set_app_paintable(eventWidget, TRUE);
> +
> +        // gtk_container_add() realizes the child widget so we need to

Better:
Before realizing the child widget by gtk_container_add we need to set if container is actually a window or not.

Comment 6

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8921201 [details]
Bug 1411018 - Rename shellHasCSD to drawToContainer and mark our rendering widget by gtk_widget_set_app_paintable(),

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192194/diff/2-3/

Comment 7

[Jan Horak [:jhorak]]

Comment on attachment 8921201 [details]
Bug 1411018 - Rename shellHasCSD to drawToContainer and mark our rendering widget by gtk_widget_set_app_paintable(),

https://reviewboard.mozilla.org/r/192194/#review199562

Looks good, thanks.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ce1b76adab5
Rename shellHasCSD to drawToContainer and mark our rendering widget by gtk_widget_set_app_paintable(), r=jhorak

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5ce1b76adab5