Bug 22560 - [GTK] Some Gtk Screen functions assume widgets are realized, leading to segfaults.
Summary: [GTK] Some Gtk Screen functions assume widgets are realized, leading to segfa...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-11-29 18:36 PST by Robert Carr
Modified: 2008-12-11 02:26 PST (History)
1 user (show)

See Also:


Attachments
Patch to fix described problem, + changelog entry. (2.22 KB, patch)
2008-11-29 18:37 PST, Robert Carr
no flags Details | Formatted Diff | Diff
Update to previous patch, fixing style mistakes. (2.24 KB, patch)
2008-11-29 20:09 PST, Timothy P. Horton
zecke: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Carr 2008-11-29 18:36:04 PST
Code in PlatformScreenGtk for screenDepth and screenRect can not
assume that the platformWindow for the widget has a valid "window" 
member, and doing so causes segfaults (screenDepth) or GLib warnings (screenRect). For example, the case of, a new browser view opening as a
child of a GtkNotebook, but never being switched to, or manually
realized. Solve by using the toplevel window of the widget, rather
than the widget itself.
Comment 1 Robert Carr 2008-11-29 18:37:07 PST
Created attachment 25603 [details]
Patch to fix described problem, + changelog entry.
Comment 2 Holger Freyther 2008-11-29 19:41:46 PST
Comment on attachment 25603 [details]
Patch to fix described problem, + changelog entry.


> -    GdkVisual* visual = gdk_drawable_get_visual(GDK_DRAWABLE(GTK_WIDGET(widget->root()->hostWindow()->platformWindow())->window));
> +	if (!GTK_WIDGET_REALIZED(container))
> +	{
> +		GtkWidget * toplevel = gtk_widget_get_toplevel(container);
> +		if (GTK_WIDGET_TOPLEVEL(toplevel))
> +			container = toplevel;
> +		else
> +			return 24;
> +	}

please see the WebKit.org CodingStyle guideleines. the '{' is on the wrong line, your indention is messed up, also the indention of the ChangeLog is messed. Please fix this and attach an updated version of the patch.
Comment 3 Timothy P. Horton 2008-11-29 20:09:50 PST
Created attachment 25605 [details]
Update to previous patch, fixing style mistakes.
Comment 4 Eric Seidel (no email) 2008-12-01 12:40:36 PST
Comment on attachment 25605 [details]
Update to previous patch, fixing style mistakes.

You can leave the Requestee field blank when requesting review.
Comment 5 Holger Freyther 2008-12-05 04:01:21 PST
Comment on attachment 25605 [details]
Update to previous patch, fixing style mistakes.


> +        Code in PlatformScreenGtk for screenDepth and screenRect can not

oh well, It can assume it. In fact the current code shows that it can assume it. It should not assume the widget to be realized. :)



> +        assume that the platformWindow for the widget has a valid "window" 
> +        member.	For example in the case of, a new browser view opening as a

remove bogus space when landing...
Comment 6 Holger Freyther 2008-12-11 02:26:54 PST
This did land in r39203.