Summary: | [WK2] Remove repeating code in declaration of WK2 API classes | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mikhail Pozdnyakov <mikhail.pozdnyakov> | ||||||
Component: | WebKit2 | Assignee: | Mikhail Pozdnyakov <mikhail.pozdnyakov> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | andersca, ap, benjamin, cmarcelo, eric.carlson, feature-media-reviews, gyuyoung.kim, jer.noble, kenneth, rakuco, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Mikhail Pozdnyakov
2013-03-20 01:19:49 PDT
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. |