[Webkit-unassigned] [Bug 16401] [GTK] GObject/C DOM binding

Mon Dec 1 02:25:53 PST 2008

https://bugs.webkit.org/show_bug.cgi?id=16401

------- Comment #97 from lkcl at lkcl.net  2008-12-01 02:25 PDT -------
(In reply to comment #96)
> Created an attachment (id=25618)
 --> (https://bugs.webkit.org/attachment.cgi?id=25618&action=view) [review]
> DOM patch updated to version 38852
> 
> So, as I said, my time is quite limited these days,

 martin, it's great that you've taken the time to work on this.

 what do you think of the idea of discussing the work that you
 intend to carry out before going ahead, to check whether the
 work fits with the future plans and direction that is needed?

 in many cases, you would be able to guess correctly, but in others
 it would perhaps take up your valuable time unnecessarily, yes?

> but I managed to make some
> progress with the patch.

 great!  can i ask you a favour?  could you produce a diff of the diff?
 i.e. using the original patch as the baseline, prepare a patch with changes
 from the original patch into the attachment 25618 patch?

 otherwise it's virtually impossible to follow along
 (which is why this 7,000 lines of complex work should be committed to
  webkit svn and then worked on, collaboratively, via webkit svn to fix
  the remaining issues).

 alternatively, martin, would you like to collaborate on this via a shared
 revision control system?

 i believe i've mentioned that i'm using github.org

> First of all, what I did so far: With the previous patch, three memory blocks
> were being allocated to wrap a single core DOM object: the *public* structure
> for the wrapping GObject, the *private* structure for the wrapping GObject, and
> an additional C++ wrapper object. I got rid of the additional C++ wrappers
> altogether, which means that the private GObject structures now point directly
> to the core objects.

 mmm... i made them that way to follow along with the conventions advised
 by various documentation and howtos that i trustingly read, but also because
 if there is anything _else_ that needs to be stored in the private gobject
 structure, the wrapper is needed.

 for example, at some point, the exception handling is going to be added back
 in (see functions in JS wrappers, they take _two_ arguments: c++ webkit obj
 and an exception object), and it will _not_ be added to the webkit webcore
 c++ objects: the only place therefore available to store and retrieve it is:

 the gobject private data.

 so - sorry to have to be the one to tell you this, but it has to stay.

> but makes the patch easier to read, because the naming scheme used for the C++
> wrappers made them very easy to confuse with the corresponding GObject classes.

 then we pick a better one.

> Additionally, while doing this, I refactored the code in many places to be more
> consistent, both in naming and behavior, with the rest of WebKitGtk.

 great - but...

 the glib / gobject patch has nothing to do with and has zero dependency on
 WebkitGtk.

> The first question I have is related to the GObject private structures. At this
> time, they contain only a pointer to the wrapped core object.

 that's right.  later they will have an exception class handling object, too.

> I also started checking the use of RefPtr and friends in the patch, and I
> currently have an issue I don't know how to solve. In file GDOMBinding.cpp, a
> number of functions called toGDOM are defined that are intended to wrap core
> DOM objects into GObjects. Since these function actually take ownership of the
> references they receive, they should use PassRefPtr's as parameters. Problem
> is, when I change the parameters, which currently are raw pointers, to
> PassRefPtr, the compiler is not able to resolve the calls any more, complaining
> that they are ambiguous.

 don't know - i'll have to take a look - but it will be on the code that
 i'm using, and used to, unless you can submit me a patch that diffs against
 this:

   http://github.com/lkcl/webkit/tree/16401.master

 not being funny or anything, but it's just _way_ too much work, otherwise,
 to be manually re-doing such massive diffs, without a common revision control
 system.

> What should I do in this case? Giving the functions
> different names based on the argument types doesn't look like an option,
> because deciding which one to call (this is mainly done by generated code) is
> not trivial.

 no, it isn't :)

 which is why i stopped work until someone paid me to continue.

> Any suggestions here would be greatly appreciated.
> 
> Finally, I would like to rename all files, manual and generated, according to
> the conventions used by the rest of WebKitGtk.

 ok - two things to emphasise: first, this is a good idea, second, this patch
 has _nothing_ to do with WebKitGtk's functionality.

 this patch is a _gobject_ patch that _happens_ to be utilised, in the first
 instance, by WebKitGtk as the first port.

 other ports - the qt and the wxWidgets - are perfectly at liberty to utilise
 the webkit-gobject bindings, by #including <glib.h>

 they DO NOT i repeat DO NOT have to #include <gtk.h>.

> This is no problem, but I'm
> wondering about the GDOM prefix.

 yeah, i don't like it, either.

> I see no reason to have a different prefix for
> the DOM related objects. They belong to WebKitGTK 

 no, they don't.  that's why alp suggested that the functionality go into
 its own library - libwebkitglib.la - to strongly emphasise this.

> as much as anything else,
> and, in my opinion, should use the same naming convention. Would people agree
> with this? Should I just change everything to use the WebKit (or webkit_
> dpending on the case) prefix?

 yep, i agree.

 _despite_ the fact that the naming convention is identical to that chosen
 by webkit's gtk port.

 it would be much better for the webkit gtk port to take a prefix of
 "WebkitGtk", and for the webkit-glib code to take a prefix of "Webkit".

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.