The following two lines of code are repeated in every class inheriting WebKit::APIObject. 1) static const Type APIType = SomeAPIType; 2) virtual Type type() const { return APIType; } This code copy/paste could be avoided.
Created attachment 194002 [details] WIP patch This proposal patch introduces template class template <APIObject::Type ArgumentType> class TypedAPIObject : public APIObject which contains the common functionality.
Comment on attachment 194002 [details] WIP patch Attachment 194002 [details] did not pass efl-ews (efl): Output: http://webkit-commit-queue.appspot.com/results/17140413
Comment on attachment 194002 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=194002&action=review > Source/WebKit2/UIProcess/efl/WebPopupItemEfl.h:35 > +class WebPopupItemEfl : TypedAPIObject<APIObject::TypePopupMenuItem> { argh, missed 'public' here :(
Comment on attachment 194002 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=194002&action=review This looks like a great idea but if you plan to only fix the ELF types, I am gonna stab you :) > Source/WebKit2/Shared/APIObject.h:165 > + virtual Type type() const { return APIType; } Shouldn't it be private?
(In reply to comment #4) > (From update of attachment 194002 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=194002&action=review > > This looks like a great idea but if you plan to only fix the ELF types, I am gonna stab you :) > No no, it was just an illustration patch, I am planning to deploy it to every object :) > > Source/WebKit2/Shared/APIObject.h:165 > > + virtual Type type() const { return APIType; } > > Shouldn't it be private? It could, but this would mean that the object cannot invoke it's own type() method, so would keep it protected.
Created attachment 194269 [details] patch
Comment on attachment 194269 [details] patch Great change! Can you deploy it to all APIObject? (WebFrameListenerProxy's subclass as you mentioned on IRC)
(In reply to comment #7) > (From update of attachment 194269 [details]) > Great change! > Can you deploy it to all APIObject? (WebFrameListenerProxy's subclass as you mentioned on IRC) Unfortunately it's not that easy to deploy to WebFrameListenerProxy's subclasses because WebFrameProxy contains RefPtr<WebFrameListenerProxy> m_activeListener; hence WebFrameListenerProxy has to inherit ThreadSafeRefCounted directly. The only solution that I see is virtual inheritance, would you like it?
Comment on attachment 194269 [details] patch > Unfortunately it's not that easy to deploy to WebFrameListenerProxy's subclasses because WebFrameProxy contains RefPtr<WebFrameListenerProxy> m_activeListener; hence WebFrameListenerProxy has to inherit ThreadSafeRefCounted directly. > The only solution that I see is virtual inheritance, would you like it? Nope, screw that :)
Comment on attachment 194269 [details] patch Clearing flags on attachment: 194269 Committed r147403: <http://trac.webkit.org/changeset/147403>
All reviewed patches have been landed. Closing bug.