Bug 16606 – [Gtk] Clean up WebKitWebFrame initialization code

• Component: WebKitGTK
• Status: RESOLVED WONTFIX

[Christian Dywan]

There is no initialization code in webkit_web_frame_init. Also webkit_web_frame_new and webkit_web_frame_init_with_web_view contain very similar code that can be merged.
Furthermore webkit_web_frame_init_with_web_view is misnamed.

[Christian Dywan]

Created attachment 18111 [details]
Cleanup and renaming

Rename webkit_web_frame_init_with_web_view to webkit_web_frame_new_with_element.

Cleanup initialization code in webkit_web_frame_init, webkit_web_frame_new and webkit_web_frame_new_with_element.

[Alp Toker]

-    frameView->deref();

What's this change for?

[Xan Lopez]

+    frameData->name = 0;
+    frameData->title = 0;
+    frameData->uri = 0;

FWIW, this is redundant. Private data is zeroed when created already.

[Christian Dywan]

Created attachment 18196 [details]
updated

[Christian Dywan]

Comment on attachment 18196 [details]
updated

Clearing review in favor of a bit more investigation.

[Leonid Romanov]

Recently, I've been investigating where and how WebKitWebFrame instance created by FrameLoaderClient::createFrame() method gets freed. Someone on irc, where I looked for help, has asked me to report my findings here, on bugzilla. So, here we go.
I used RefDbg tool to trace GObjects creation/refs/derefs/finalization and according to it, the only WebKitWebFrame instance that gets freed is the main frame. Other frames, created via webkit_web_frame_init_with_web_view are never freed. 
Here what I did:
- Download and compile RefDbg from http://refdbg.sourceforge.net/
- Compile debug version of WebKit GTK
- Start GtkLauncher under RefDbg with http://www.w3schools.com/html/tryit.asp?filename=tryhtml_frame_cols as page to open
- Wait till page fully loads and then close GtkLauncher.
- Examine RefDbg log for leaks 

[Martin Robinson]

I just ran this test in a recent build of GtkLauncher. I couldn't get RefDbg working, so I just put print statements in the WebKitWebFrame finalization method. If I quit GtkLauncher on the test page, there were many frames that were not finalized, but if I left the page for a blank page before exiting (by accessing a non-existent address), only the main frame was not finalized. This doesn't appear to be an issue any longer.

[Chris Leonard]

Can it be conformed whether or not thsi is still a current issue and if not can this ticket be clsoed?

[Martin Robinson]

(In reply to comment #8)
> Can it be conformed whether or not thsi is still a current issue and if not can this ticket be clsoed?

Well, the original bug isn't about the RefDbg issue you mentioned in comment #6, so do you mean the original cleanup or the issue you mentioned?

[Martin Robinson]

WebKit1GTK+ has been removed.