[Webkit-unassigned] [Bug 31737] Chromium WebKit API WebPageSerializer

[bugzilla-daemon at webkit.org]

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


Darin Fisher (:fishd, Google) <fishd at chromium.org> changed:

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




--- Comment #11 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2009-11-30 12:50:37 PST ---
(From update of attachment 44029)
> +++ b/WebKit/chromium/public/WebNodeCollection.h
...
> +class WebNodeCollection {
> +public:
> +    ~WebNodeCollection() { reset(); }
> +
> +    WebNodeCollection(const WebNodeCollection& n) : m_private(0) { assign(n); }
> +    WebNodeCollection& operator=(const WebNodeCollection& n)
> +    {
> +        assign(n);
> +        return *this;
> +    }
> +
> +    WEBKIT_API void reset();
> +    WEBKIT_API void assign(const WebNodeCollection&);
> +
> +    WEBKIT_API unsigned length() const;
> +    WEBKIT_API WebNode nextItem() const;
> +    WEBKIT_API WebNode firstItem() const;
> +
> +#if WEBKIT_IMPLEMENTATION
> +    WebNodeCollection(const WTF::PassRefPtr<WebCore::HTMLCollection>&);
> +#endif
> +
> +private:
> +    void assign(WebCore::HTMLCollection*);
> +    WebCore::HTMLCollection* m_private;
> +};

It looks like this class is missing a default constructor.  I think that'll
cause some crashes.


> +++ b/WebKit/chromium/public/WebNodeList.h
...
> +class WebNodeList {
> +public:
> +    ~WebNodeList() { reset(); }
> +
> +    WebNodeList(const WebNodeList& n) : m_private(0) { assign(n); }
> +    WebNodeList& operator=(const WebNodeList& n)
> +    {
> +        assign(n);
> +        return *this;
> +    }
> +
> +    WEBKIT_API void reset();
> +    WEBKIT_API void assign(const WebNodeList&);
> +
> +    WEBKIT_API unsigned length() const;
> +    WEBKIT_API WebNode item(size_t) const;
> +
> +#if WEBKIT_IMPLEMENTATION
> +    WebNodeList(const WTF::PassRefPtr<WebCore::NodeList>&);
> +#endif
> +
> +private:
> +    void assign(WebCore::NodeList*);
> +    WebCore::NodeList* m_private;
> +};

ditto



> +++ b/WebKit/chromium/public/WebPageSerializerClient.h
...
> +    enum PageSerializationStatus {
> +        CurrentFrameIsNotFinished = 0,

See previous nit about removing the "= 0"...


> +++ b/WebKit/chromium/public/WebString.h
> @@ -87,6 +87,9 @@ public:
>      WEBKIT_API static WebString fromUTF8(const char* data, size_t length);
>      WEBKIT_API static WebString fromUTF8(const char* data);
>  
> +    WebString(const char* data);
> +    WebString& operator=(const char* s);

looks like this is the same as before.  did you update the wrong patch?


> +++ b/WebKit/chromium/src/WebNode.cpp
...
> +WebNode::NodeType WebNode::nodeType() const
> +{
> +    return static_cast<WebNode::NodeType>(m_private->nodeType());
> +}

see previous nit about dropping the WebNode:: prefix within the body
of a WebNode method.


> +++ b/WebKit/chromium/src/WebNodeList.cpp
...
> +#include "NodeList.h"
> +#include "Node.h"
> +
> +#include "WebNode.h"
> +
> +#include <wtf/PassRefPtr.h>

see previous nit about moving the wtf header up to the section
including Node.h

[stopping here since i'm repeating myself... is this the right patch?]

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