[webkit-reviews] review requested: [Bug 30822] Change NativeImagePtr definition for wx port : [Attachment 41954] Patch changing NativeImagePtr definition for wx port

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 27 08:11:43 PDT 2009


Vadim Zeitlin <vz-webkit at zeitlins.org> has asked  for review:
Bug 30822: Change NativeImagePtr definition for wx port
https://bugs.webkit.org/show_bug.cgi?id=30822

Attachment 41954: Patch changing NativeImagePtr definition for wx port
https://bugs.webkit.org/attachment.cgi?id=41954&action=review

------- Additional Comments from Vadim Zeitlin <vz-webkit at zeitlins.org>
This patch changes NativeImagePtr definition from a pointer to
wx[Graphics]Bitmap to just the object itself for wx port. The reason for doing
this is that these classes are already smart pointer-like in wx and it just
doesn't make sense to have another layer of indirection and extra heap
allocations when using them -- they're supposed to be passed around by value.

I didn't try to build any other ports after this change but I hope to not
having broken them as the definitions of the types didn't change for them so
the existing code should continue to work and the changes to common,
port-independent, parts were tested as part of wxWebKit rebuild.

Finally, this is, of course, hardly a critical issue and it does have some
drawbacks: for one, I don't know if making a type with "Ptr" name not a pointer
is acceptable. Second, the common code should take care to use
"NativeImagePtr()" instead of "NULL" when initializing invalid native image
objects and use the new "IsValidNativeImagePtr()" function for testing them
instead of just comparing with "NULL" (although the latter can still be done in
port-specific code for ports other than wx, of course). But I still hope this
patch can be applied because allocating these objects on the heap is just
unnecessary and the code doing it is painful to read.

TIA!


More information about the webkit-reviews mailing list