[Webkit-unassigned] [Bug 47394] [chromium] Update ReadableDataObject/WritableDataObject interface for ChromiumDataObject change

[bugzilla-daemon at webkit.org]

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


Tony Chang <tony at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #70196|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #2 from Tony Chang <tony at chromium.org>  2010-10-08 10:10:29 PST ---
(From update of attachment 70196)
View in context: https://bugs.webkit.org/attachment.cgi?id=70196&action=review

> WebCore/platform/chromium/ReadableDataObject.h:54
> -    virtual String getHTML(String* baseURL) const;
> +    String urlTitle() const;
> +    KURL htmlBaseUrl() const;

I prefer the getHTML method because it makes the caller think about whether they need the base url or not.  I feel less strongly about the getURL method, but it's pretty easy to pass in NULL for the title so I'm not sure there's much benefit to splitting it up.

> WebCore/platform/chromium/WritableDataObject.h:59
> -    virtual void setURL(const String& url, const String& title);
> -    virtual void setHTML(const String& html, const KURL& baseURL);
> +    void setUrlTitle(const String& title);
> +    void setHtmlBaseUrl(const KURL& baseURL);

I prefer the old API where you can set everything in a single function call.  It makes it less likely that someone forgets to call setUrlTitle or setHtmlBaseUrl.  Also, in webkit, we tend to capitalize URL (e.g., KURL) and HTML (e.g., all the files in WebCore/html/).

> WebCore/platform/chromium/WritableDataObject.h:64
>      virtual String urlTitle() const;
>      virtual KURL htmlBaseURL() const;

Are these methods actually called?  Do they need to be virtual?

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