[Webkit-unassigned] [Bug 28037] SocketStreamHandle interface for WebSocket API

[bugzilla-daemon at webkit.org]

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





--- Comment #2 from David Levin <levin at chromium.org>  2009-08-11 08:38:47 PDT ---
(From update of attachment 34545)
Simply a style pass. I looked at this and saw a few style issues to clean up.

> diff --git a/WebCore/platform/network/SocketStreamErrorBase.h b/WebCore/platform/network/SocketStreamErrorBase.h
> +    class SocketStreamErrorBase {
> +      public:

public:, private:, protected: should be aligned with "class". There are
multiple instances but I'm only commenting on this one.

> +        SocketStreamErrorBase()
> +                : m_errorCode(0)
> +                , m_isNull(true)

These should only be indented 4 spaces (not 8). There are multiple instances
but I'm only commenting on this one.


> diff --git a/WebCore/platform/network/SocketStreamHandle.h b/WebCore/platform/network/SocketStreamHandle.h
> +        virtual int platformSend(const char*, int);

Please include parameter names for this method.
Parameter names are omitted when they don't add information.  In this case,
they would add information. 


> diff --git a/WebCore/platform/network/SocketStreamHandleBase.cpp b/WebCore/platform/network/SocketStreamHandleBase.cpp
> +bool SocketStreamHandleBase::send(const char* buf, int size)
s/buf/buffer/
Use full words for variables.

> +    int num_written = 0;

s/num_written/bytesWritten/

bad casing.

> +void SocketStreamHandleBase::setClient(SocketStreamHandleClient* client)
> +{

Consider adding asserts here.
Can this be called in any state?
Can it be called after a client has already been set?


> +bool SocketStreamHandleBase::sendPendingData()
> +{
> +    if (m_state != Open)
> +        return false;
> +    if (m_buffer.isEmpty())
> +        return false;
> +    int num_written = platformSend(m_buffer.data(), m_buffer.size());

s/num_written/bytesWritten/

bad casing.

> +    Vector<char> remaining_data;

s/remaining_data/remainingData/

bad casing.

> +    remaining_data.append(m_buffer.data() + num_written,
> +                           m_buffer.size() - num_written);

Indentation mistake.


> diff --git a/WebCore/platform/network/SocketStreamHandleBase.h b/WebCore/platform/network/SocketStreamHandleBase.h
> +        virtual ~SocketStreamHandleBase() {}
s/{}/{ }/
Add a space in the {}


> +        bool send(const char*, int);

Please include parameter names for this method.

> +        virtual int platformSend(const char*, int) = 0;

Please include parameter names for this method.

> diff --git a/WebCore/platform/network/SocketStreamHandleClient.h b/WebCore/platform/network/SocketStreamHandleClient.h
> +        virtual void willSendData(SocketStreamHandle*, const char*, int) { }

Please include parameter names for the char* and int.

> +        virtual void didReceivedData(SocketStreamHandle*, const char*, int) { }

Please include parameter names for the char* and int.


> diff --git a/WebCore/platform/network/mac/SocketStreamError.h b/WebCore/platform/network/mac/SocketStreamError.h
> +        SocketStreamError() {}
s/{}/{ }/

> diff --git a/WebCore/platform/network/mac/SocketStreamHandleMac.mm b/WebCore/platform/network/mac/SocketStreamHandleMac.mm
> +    SocketStreamHandleInternal() {}
> +    ~SocketStreamHandleInternal() {}

s/{}/{ }/


> diff --git a/WebCore/platform/network/soup/SocketStreamHandleSoup.cpp b/WebCore/platform/network/soup/SocketStreamHandleSoup.cpp
> +class SocketStreamHandleInternal {
> +public:
> +    SocketStreamHandleInternal() {}
> +    ~SocketStreamHandleInternal() {}

s/{}/{ }/

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