Bug 45550

Summary: [GTK] Placement new / manual destructor invocation should be used on private GObject memory
Product: WebKit Reporter: Martin Robinson <mrobinson>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, gustavo, mrobinson, webkit.review.bot, xan.lopez
Priority: P3 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 21594    
Attachments:
Description Flags
Patch making this change for WebKitWebView none

Description Martin Robinson 2010-09-10 10:31:52 PDT 
GLib allocates and deallocates GObject private data structs itself. When those structs contain C++ members, their constructors and destructors are not called. This is not only dangerous, it makes RefPtr-type smart pointers much less useful. We can fix this problem by calling placement new on the private data struct during instance initialization and calling the destructor during finalization.
Comment 1 Martin Robinson 2010-09-10 10:49:58 PDT 
Created attachment 67200 [details]
Patch making this change for WebKitWebView
Comment 2 Xan Lopez 2010-09-10 10:57:48 PDT 
Comment on attachment 67200 [details]
Patch making this change for WebKitWebView


>-    if (priv->tooltipText) {
>-        gtk_tooltip_set_text(tooltip, priv->tooltipText);
>+    if (priv->tooltipText.length() > 0) {

Mmm, should use isEmpty() for consistency sake?

>+        gtk_tooltip_set_text(tooltip, priv->tooltipText.data());
>         return TRUE;
>     }
> 


Looks great otherwise.
Comment 3 Martin Robinson 2010-09-10 11:02:35 PDT 
(In reply to comment #2)

Thanks for the review!

> Mmm, should use isEmpty() for consistency sake?

CString doesn't have an isEmpty unfortunately. I could see it being useful though. Perhaps I'll propose that in another patch.
Comment 4 Martin Robinson 2010-09-10 11:05:05 PDT 
Comment on attachment 67200 [details]
Patch making this change for WebKitWebView

Clearing flags on attachment: 67200

Committed r67215: <http://trac.webkit.org/changeset/67215>
Comment 5 Martin Robinson 2010-09-10 11:05:09 PDT 
All reviewed patches have been landed.  Closing bug.