Bug 43163

Summary: Add a CF-style base type (WKTypeRef) as a base for polymorphic functions
Product: WebKit Reporter: Sam Weinig <sam>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch darin: review+

Description Sam Weinig 2010-07-28 17:40:57 PDT
Add a CF-style base type (WKTypeRef) as a base for polymorphic functions.
Comment 1 Sam Weinig 2010-07-28 17:41:52 PDT
Created attachment 62900 [details]
Patch
Comment 2 WebKit Review Bot 2010-07-28 17:44:10 PDT
Attachment 62900 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKType.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit2/UIProcess/API/C/WKAPICast.h:54:  More than one command on the same line  [whitespace/newline] [4]
WebKit2/UIProcess/API/C/WKAPICast.h:61:  More than one command on the same line  [whitespace/newline] [4]
WebKit2/UIProcess/API/C/WKAPICast.h:62:  More than one command on the same line  [whitespace/newline] [4]
WebKit2/UIProcess/API/C/WKAPICast.h:69:  More than one command on the same line  [whitespace/newline] [4]
WebKit2/UIProcess/API/C/WKAPICast.h:77:  More than one command on the same line  [whitespace/newline] [4]
WebKit2/UIProcess/API/C/WKAPICast.h:124:  Extra space before last semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 7 in 54 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Darin Adler 2010-07-28 17:47:26 PDT
Comment on attachment 62900 [details]
Patch

> +    virtual Type type() const { return TypeArray; }

> +    virtual Type type() const { return TypeString; }

> +    virtual Type type() const { return TypeURL; }

> +    virtual Type type() const { return TypeBackForwardList; }

> +    virtual Type type() const { return TypeBackForwardListItem; }

> +    virtual Type type() const { return TypeContext; }

> +    virtual Type type() const { return TypeFramePolicyListener; }

> +    virtual Type type() const { return TypeFrame; }

> +    virtual Type type() const { return TypeNavigationData; }

> +    virtual Type type() const { return TypePageNamespace; }

> +    virtual Type type() const { return TypePage; }

> +    virtual Type type() const { return TypePreferences; }

> +    virtual Type type() const { return TypeBundleFrame; }

> +    virtual Type type() const { return TypeBundlePage; }

Making this private may catch some callers accidentally calling the virtual function on something already known to be a WebArray.

> +    operator APIType() { return reinterpret_cast<APIType>(m_impl.get()); }

Doesn't static_cast work for this? If not, why not?
Comment 4 Sam Weinig 2010-07-28 18:00:06 PDT
(In reply to comment #3)
> (From update of attachment 62900 [details])
> > +    virtual Type type() const { return TypeArray; }
> 
> > +    virtual Type type() const { return TypeString; }
> 
> > +    virtual Type type() const { return TypeURL; }
> 
> > +    virtual Type type() const { return TypeBackForwardList; }
> 
> > +    virtual Type type() const { return TypeBackForwardListItem; }
> 
> > +    virtual Type type() const { return TypeContext; }
> 
> > +    virtual Type type() const { return TypeFramePolicyListener; }
> 
> > +    virtual Type type() const { return TypeFrame; }
> 
> > +    virtual Type type() const { return TypeNavigationData; }
> 
> > +    virtual Type type() const { return TypePageNamespace; }
> 
> > +    virtual Type type() const { return TypePage; }
> 
> > +    virtual Type type() const { return TypePreferences; }
> 
> > +    virtual Type type() const { return TypeBundleFrame; }
> 
> > +    virtual Type type() const { return TypeBundlePage; }
> 
> Making this private may catch some callers accidentally calling the virtual function on something already known to be a WebArray.

Will do.

> > +    operator APIType() { return reinterpret_cast<APIType>(m_impl.get()); }
> 
> Doesn't static_cast work for this? If not, why not?

/Volumes/Data/Users/weinig/Code/webkit/OpenSource/WebKit2/UIProcess/API/C/WKAPICast.h:91: error: invalid static_cast from type 'WebKit::WebString*' to type 'OpaqueWKStringRef*'
Comment 5 Sam Weinig 2010-07-28 21:52:57 PDT
Landed in http://trac.webkit.org/changeset/64253.