Bug 34180 – [Qt] Enable websockets support in QtWebKit

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Yael]

A patch is coming soon

[Yael]

(In reply to comment #0)
> A patch is coming soon

Unfortunately, I am getting a couple of sparious crashes when running websockets tests in DRT. Need more time to investigate the crashes.

[Yael]

Created attachment 47474 [details]
Patch v1

About 2 out of the 27 websockets tests in DRT are crashing randomly with this patch, but I wanted to start the review process while I am working on the crashes.
With this patch 
- both secure and non-secure websockets work
- SSL errors are ignored
- proxy authentication is not supported.

[Yael]

Created attachment 47476 [details]
Patch v2

Got rid of all the tabs in the code. Maybe eclipse is not the best editor after all :-)

[Fumitoshi Ukai]

Comment on attachment 47476 [details]
Patch v2

> Index: WebCore/WebCore.pro
> ===================================================================
> --- WebCore/WebCore.pro	(revision 53851)
> +++ WebCore/WebCore.pro	(working copy)
> @@ -1530,6 +1530,7 @@
>      platform/network/NetworkStateNotifier.h \
>      platform/network/ProtectionSpace.h \
>      platform/network/qt/QNetworkReplyHandler.h \
> +    platform/network/qt/SocketStreamHandlePrivate.h \
>      platform/network/ResourceErrorBase.h \
>      platform/network/ResourceHandle.h \
>      platform/network/ResourceRequestBase.h \
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 53876)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,34 @@
> [phone]  Yael Aharon  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [Qt] Enable websockets support in QtWebKit
> +        https://bugs.webkit.org/show_bug.cgi?id=34180
> +
> +        Connected between SocketStreamHandle and QTcpSocket/QSslSocket
> +        to enable support for websockets.
> +
> +        * WebCore.pro:
> +        * platform/network/qt/SocketStreamHandle.h:
> +        * platform/network/qt/SocketStreamHandlePrivate.h: Added.
> +        * platform/network/qt/SocketStreamHandleQt.cpp:
> +        (WebCore::SocketStreamHandlePrivate::SocketStreamHandlePrivate):
> +        (WebCore::SocketStreamHandlePrivate::~SocketStreamHandlePrivate):
> +        (WebCore::SocketStreamHandlePrivate::socketConnected):
> +        (WebCore::SocketStreamHandlePrivate::socketReadyRead):
> +        (WebCore::SocketStreamHandlePrivate::send):
> +        (WebCore::SocketStreamHandlePrivate::close):
> +        (WebCore::SocketStreamHandlePrivate::socketSentdata):
> +        (WebCore::SocketStreamHandlePrivate::socketClosed):
> +        (WebCore::SocketStreamHandlePrivate::socketError):
> +        (WebCore::SocketStreamHandlePrivate::socketClosedCallback):
> +        (WebCore::SocketStreamHandlePrivate::socketErrorCallback):
> +        (WebCore::SocketStreamHandlePrivate::socketSslErrors):
> +        (WebCore::SocketStreamHandle::SocketStreamHandle):
> +        (WebCore::SocketStreamHandle::~SocketStreamHandle):
> +        (WebCore::SocketStreamHandle::platformSend):
> +        (WebCore::SocketStreamHandle::platformClose):
> +
>  2010-01-26  Dmitry Titov  <[e-mail]>
>  
>          Reviewed by Steve Falkenburg.
> Index: WebCore/platform/network/qt/SocketStreamHandlePrivate.h
> ===================================================================
> --- WebCore/platform/network/qt/SocketStreamHandlePrivate.h	(revision 0)
> +++ WebCore/platform/network/qt/SocketStreamHandlePrivate.h	(revision 0)
> @@ -0,0 +1,74 @@
> +/*
> + * Copyright (C) 2009 Apple Inc. All rights reserved.
> + * Copyright (C) 2009 Google Inc.  All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are
> + * met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above
> + * copyright notice, this list of conditions and the following disclaimer
> + * in the documentation and/or other materials provided with the
> + * distribution.
> + *     * Neither the name of Google Inc. nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef SocketStreamHandlePrivate_h
> +#define SocketStreamHandlePrivate_h
> +
> +#include "SocketStreamHandleBase.h"
> +
> +#include <QSslSocket>
> +#include <QTcpSocket>
> +
> +class QSslSocket;
> +
> +namespace WebCore {
> +
> +class AuthenticationChallenge;
> +class Credential;
> +class SocketStreamHandleClient;
> +class SocketStreamHandlePrivate;
> +
> +class SocketStreamHandlePrivate : public QObject {
> +Q_OBJECT
> +public:
> +    SocketStreamHandlePrivate(SocketStreamHandle* streamHandle, const KURL& url);
> +    ~SocketStreamHandlePrivate();
> +
> +public slots:
> +    void socketConnected();
> +    void socketReadyRead();
> +    int send(const char* data, int len);
> +    void close();
> +    void socketSentdata();
> +    void socketClosed();
> +    void socketError(QAbstractSocket::SocketError);
> +    void socketClosedCallback();
> +    void socketErrorCallback(int);
> +    void socketSslErrors(const QList<QSslError>&);
> +public:
> +    QTcpSocket* m_socket;
> +    SocketStreamHandle* m_streamHandle;
> +    quint64 m_lastSentSize;

Do you need this?

> +};
> +
> +}
> +
> +#endif
> Index: WebCore/platform/network/qt/SocketStreamHandle.h
> ===================================================================
> --- WebCore/platform/network/qt/SocketStreamHandle.h	(revision 53851)
> +++ WebCore/platform/network/qt/SocketStreamHandle.h	(working copy)
> @@ -42,6 +42,7 @@
>      class AuthenticationChallenge;
>      class Credential;
>      class SocketStreamHandleClient;
> +    class SocketStreamHandlePrivate;
>  
>      class SocketStreamHandle : public RefCounted<SocketStreamHandle>, public SocketStreamHandleBase {
>      public:
> @@ -61,6 +62,8 @@
>          void receivedCredential(const AuthenticationChallenge&, const Credential&);
>          void receivedRequestToContinueWithoutCredential(const AuthenticationChallenge&);
>          void receivedCancellation(const AuthenticationChallenge&);
> +        SocketStreamHandlePrivate* m_p;
> +        friend class SocketStreamHandlePrivate;
>      };
>  
>  }  // namespace WebCore
> Index: WebCore/platform/network/qt/SocketStreamHandleQt.cpp
> ===================================================================
> --- WebCore/platform/network/qt/SocketStreamHandleQt.cpp	(revision 53851)
> +++ WebCore/platform/network/qt/SocketStreamHandleQt.cpp	(working copy)
> @@ -35,34 +35,128 @@
>  #include "Logging.h"
>  #include "NotImplemented.h"
>  #include "SocketStreamHandleClient.h"
> +#include "SocketStreamHandlePrivate.h"
>  
>  namespace WebCore {
>  
> +SocketStreamHandlePrivate::SocketStreamHandlePrivate(SocketStreamHandle* streamHandle, const KURL& url) : QObject()
> +{
> +    m_streamHandle = streamHandle;
> +    m_socket = 0;
> +    bool isSecure = url.protocol().contains("wss");

url.protocolIs("wss") ?

> +    if (isSecure)
> +        m_socket = new QSslSocket(this);
> +    else
> +        m_socket = new QTcpSocket(this);
> +    connect(m_socket, SIGNAL(connected()), this, SLOT(socketConnected()));
> +    connect(m_socket, SIGNAL(readyRead()), this, SLOT(socketReadyRead()));
> +    connect(m_socket, SIGNAL(disconnected()), this, SLOT(socketClosed()));
> +    connect(m_socket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(socketError(QAbstractSocket::SocketError)));
> +    if (isSecure)
> +        connect(m_socket, SIGNAL(sslErrors(const QList<QSslError>&)), this, SLOT(socketSslErrors(const QList<QSslError>&)));
> +    unsigned int port = url.hasPort() ? url.port() : isSecure ? 443 : 80;
> +    QString host = url.host();
> +    if (isSecure)
> +        qobject_cast<QSslSocket*>(m_socket)->connectToHostEncrypted(host, port);
> +    else
> +        m_socket->connectToHost(host, port);
> +}
> +
> +SocketStreamHandlePrivate::~SocketStreamHandlePrivate()
> +{
> +    Q_ASSERT(!(m_socket && m_socket->state() == QAbstractSocket::ConnectedState));

m_socket is leaked?

> +}
> +
> +void SocketStreamHandlePrivate::socketConnected()
> +{
> +    if (m_streamHandle && m_streamHandle->client()) {
> +        m_streamHandle->m_state = SocketStreamHandleBase::Open;
> +        m_streamHandle->client()->didOpen(m_streamHandle);
> +    }
> +}
> +
> +void SocketStreamHandlePrivate::socketReadyRead()
> +{
> +    if (m_streamHandle && m_streamHandle->client()) {
> +        QByteArray data = m_socket->readAll();
> +        m_streamHandle->client()->didReceiveData(m_streamHandle, data.constData(), data.size());
> +    }
> +}
> +
> +int SocketStreamHandlePrivate::send(const char* data, int len)
> +{
> +    m_lastSentSize =  m_socket->write(data, len);
> +    QMetaObject::invokeMethod(this, "socketSentData", Qt::QueuedConnection);
> +    return m_lastSentSize;
> +}
> +
> +void SocketStreamHandlePrivate::close()
> +{
> +    if (m_socket && m_socket->state() == QAbstractSocket::ConnectedState)
> +        m_socket->close();
> +}
> +
> +void SocketStreamHandlePrivate::socketSentdata()
> +{
> +    if (m_streamHandle)
> +        m_streamHandle->sendPendingData();
> +}
> +
> +void SocketStreamHandlePrivate::socketClosed()
> +{
> +    QMetaObject::invokeMethod(this, "socketClosedCallback", Qt::QueuedConnection);
> +}
> +
> +void SocketStreamHandlePrivate::socketError(QAbstractSocket::SocketError error)
> +{
> +    QMetaObject::invokeMethod(this, "socketError", Qt::QueuedConnection, Q_ARG(int, error));
> +}
> +
> +void SocketStreamHandlePrivate::socketClosedCallback()
> +{
> +    if (m_streamHandle && m_streamHandle->client()) {
> +        m_streamHandle->client()->didClose(m_streamHandle);
> +        m_streamHandle = 0;
> +    }
> +}
> +
> +void SocketStreamHandlePrivate::socketErrorCallback(int error)
> +{
> +    // FIXME - in the future, we might not want to treat all errors as fatal.
> +    if (m_streamHandle && m_streamHandle->client()) {
> +        m_streamHandle->client()->didClose(m_streamHandle);
> +        m_streamHandle = 0;
> +    }
> +}
> +
> +void SocketStreamHandlePrivate::socketSslErrors(const QList<QSslError>&)
> +{
> +    qobject_cast<QSslSocket*>(m_socket)->ignoreSslErrors();
> +}
>  SocketStreamHandle::SocketStreamHandle(const KURL& url, SocketStreamHandleClient* client)
>      : SocketStreamHandleBase(url, client)
>  {
>      LOG(Network, "SocketStreamHandle %p new client %p", this, m_client);
> -    notImplemented();
> +    m_p = new SocketStreamHandlePrivate(this, url);
>  }
>  
>  SocketStreamHandle::~SocketStreamHandle()
>  {
>      LOG(Network, "SocketStreamHandle %p delete", this);
>      setClient(0);
> -    notImplemented();
> +    delete m_p;
>  }
>  
> -int SocketStreamHandle::platformSend(const char*, int)
> +int SocketStreamHandle::platformSend(const char* data, int len)
>  {
>      LOG(Network, "SocketStreamHandle %p platformSend", this);
> -    notImplemented();
> -    return 0;
> +    return m_p->send(data, len);
>  }
>  
>  void SocketStreamHandle::platformClose()
>  {
>      LOG(Network, "SocketStreamHandle %p platformClose", this);
> -    notImplemented();
> +    m_p->close();
>  }
>  
>  void SocketStreamHandle::didReceiveAuthenticationChallenge(const AuthenticationChallenge&)
> @@ -86,3 +180,5 @@
>  }
>  
>  } // namespace WebCore
> +
> +#include "moc_SocketStreamHandlePrivate.cpp"
> Index: LayoutTests/platform/qt/Skipped
> ===================================================================
> --- LayoutTests/platform/qt/Skipped	(revision 53851)
> +++ LayoutTests/platform/qt/Skipped	(working copy)
> @@ -4965,7 +4965,6 @@
>  http/tests/xmlhttprequest/workers/shared-worker-methods-async.html
>  
>  # Missing SocketStreamHandle implementation
> -websocket/tests
>  
>  # Need rebaseline: https://bugs.webkit.org/show_bug.cgi?id=26830
>  fast/multicol/single-line.html
> Index: LayoutTests/ChangeLog
> ===================================================================
> --- LayoutTests/ChangeLog	(revision 53876)
> +++ LayoutTests/ChangeLog	(working copy)
> @@ -1,3 +1,14 @@
> [phone]  Yael Aharon  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [Qt] Enable websockets support in QtWebKit
> +        https://bugs.webkit.org/show_bug.cgi?id=34180
> +
> +        Remove websockets tests from skiplist.
> +
> +        * platform/qt/Skipped:
> +
>  2010-01-26  Fumitoshi Ukai  <[e-mail]>
>  
>          Reviewed by Alexey Proskuryakov.

[Simon Hausmann]

Comment on attachment 47476 [details]
Patch v2

Great that you're starting with this, Yael!


> +#include <QSslSocket>
> +#include <QTcpSocket>
> +
> +class QSslSocket;

Why include QSslSocket and also forward declare it? :)

> +class SocketStreamHandlePrivate : public QObject {
> +Q_OBJECT

The Q_OBJECT macro us usually indented.

> +public:
> +    SocketStreamHandlePrivate(SocketStreamHandle* streamHandle, const KURL& url);

WebKit style suggests to remove the names of the arguments here, as they're redundant (The type is the same as the variable name).

> +    connect(m_socket, SIGNAL(connected()), this, SLOT(socketConnected()));
> +    connect(m_socket, SIGNAL(readyRead()), this, SLOT(socketReadyRead()));
> +    connect(m_socket, SIGNAL(disconnected()), this, SLOT(socketClosed()));
> +    connect(m_socket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(socketError(QAbstractSocket::SocketError)));
> +    if (isSecure)
> +        connect(m_socket, SIGNAL(sslErrors(const QList<QSslError>&)), this, SLOT(socketSslErrors(const QList<QSslError>&)));
> +    unsigned int port = url.hasPort() ? url.port() : isSecure ? 443 : 80;

This looks rather convoluted :). Do you mind splitting this up a little bit or adding parentheses?

> +    QString host = url.host();
> +    if (isSecure)
> +        qobject_cast<QSslSocket*>(m_socket)->connectToHostEncrypted(host, port);

I suggest to do a static_cast here, it's faster and if something is wrong it'll crash either way ;-)

> +void SocketStreamHandlePrivate::socketReadyRead()
> +{
> +    if (m_streamHandle && m_streamHandle->client()) {
> +        QByteArray data = m_socket->readAll();
> +        m_streamHandle->client()->didReceiveData(m_streamHandle, data.constData(), data.size());
> +    }
> +}

Is it intentional to read _all_ data here and block until all is available?

I wonder if it should be done like in QNetworkReplyHandler instead:

    QByteArray data = m_socket->read(m_socket->bytesAvailable());


Performance wise it's sad that we have to copy the data here, but I don't see any obvious solution here (as well as in QNetworkReplyHandler).

> +int SocketStreamHandlePrivate::send(const char* data, int len)
> +{
> +    m_lastSentSize =  m_socket->write(data, len);

Looks like one whitespace too much here :)

> +    QMetaObject::invokeMethod(this, "socketSentData", Qt::QueuedConnection);

What's the reason for this queued connection? Is the goal to call sendPendingData() only once the previous data has been written? (That itself may not be guaranteed with a queued connection that just goes through the event loop once)

> +void SocketStreamHandlePrivate::socketClosed()
> +{
> +    QMetaObject::invokeMethod(this, "socketClosedCallback", Qt::QueuedConnection);
> +}
> +
> +void SocketStreamHandlePrivate::socketError(QAbstractSocket::SocketError error)
> +{
> +    QMetaObject::invokeMethod(this, "socketError", Qt::QueuedConnection, Q_ARG(int, error));
> +}

Out of curiousity again, why the queued connections?


> +void SocketStreamHandlePrivate::socketErrorCallback(int error)
> +{
> +    // FIXME - in the future, we might not want to treat all errors as fatal.
> +    if (m_streamHandle && m_streamHandle->client()) {
> +        m_streamHandle->client()->didClose(m_streamHandle);

Since this is called in case of socket errors, shouldn't we call didFail() on the stream handle client instead of just didClose()?


> +void SocketStreamHandlePrivate::socketSslErrors(const QList<QSslError>&)
> +{
> +    qobject_cast<QSslSocket*>(m_socket)->ignoreSslErrors();
> +}

Hmmm, doesn't that effectively remove any security from encrypted connections? If someone spoofs a certificate we ignore it and still happily connect, allowing a man-in-the-middle to read all the traffic.


> Index: LayoutTests/platform/qt/Skipped
> ===================================================================
> --- LayoutTests/platform/qt/Skipped	(revision 53851)
> +++ LayoutTests/platform/qt/Skipped	(working copy)
> @@ -4965,7 +4965,6 @@
>  http/tests/xmlhttprequest/workers/shared-worker-methods-async.html
>  
>  # Missing SocketStreamHandle implementation
> -websocket/tests

I like this part! Yay for layout testing :)

But you should also remove the comment then that says the implementation is missing ;-)

[Yael]

Thank you Ukai & Simon for reviewing this patch!
(In reply to comment #4)

>Do you need this?
I thought I would need it, but it turns out that I don't. Wiill take it out

>url.protocolIs("wss") ?
will do

>m_socket is leaked?
The QObject system will delete it when its parent is deleted (SocketStreamHandlePrivate)


(In reply to comment #5)
>Why include QSslSocket and also forward declare it? :)
I should take it out!

>The Q_OBJECT macro us usually indented.
will do

>WebKit style suggests to remove the names of the arguments here, as they're
>redundant (The type is the same as the variable name).
will do

>This looks rather convoluted :). Do you mind splitting this up a little bit or
>adding parentheses?
will do

>> +    QString host = url.host();
>> +    if (isSecure)
>> +        qobject_cast<QSslSocket*>(m_socket)->connectToHostEncrypted(host, port);

>I suggest to do a static_cast here, it's faster and if something is wrong it'll
>crash either way ;-)
will do

>Is it intentional to read _all_ data here and block until all is available?
>
>I wonder if it should be done like in QNetworkReplyHandler instead:
>
>    QByteArray data = m_socket->read(m_socket->bytesAvailable());
>
>Performance wise it's sad that we have to copy the data here, but I don't see
>any obvious solution here (as well as in QNetworkReplyHandler).
I thought that the result of readAll() and bytesAvailable(0 would be the same, but I will change that.

>> +    m_lastSentSize =  m_socket->write(data, len);
>
>Looks like one whitespace too much here :)
Should be added to chek-webkit-style :-)

>> +    QMetaObject::invokeMethod(this, "socketSentData", Qt::QueuedConnection);
>
>What's the reason for this queued connection? Is the goal to call
>sendPendingData() only once the previous data has been written? (That itself
>may not be guaranteed with a queued connection that just goes through the event
>loop once)
I wanted to match chromium port that calls this asynchronously. QTcpSocket does not provide a callback when it is really done sending.

>> +void SocketStreamHandlePrivate::socketClosed()
>> +{
> +    QMetaObject::invokeMethod(this, "socketClosedCallback", Qt::QueuedConnection);
>> +}
>> +
>> +void SocketStreamHandlePrivate::socketError(QAbstractSocket::SocketError error)
>> +{
>> +    QMetaObject::invokeMethod(this, "socketError", Qt::QueuedConnection, Q_ARG(int, error));
>> +}
>
>Out of curiousity again, why the queued connections?
These calls would result in cleanup, and I thought it was safer to call them asynchronously. Do you think this will cause performance issues?

>> +void SocketStreamHandlePrivate::socketErrorCallback(int error)
>> +{
>> +    // FIXME - in the future, we might not want to treat all errors as fatal.
>> +    if (m_streamHandle && m_streamHandle->client()) {
>> +        m_streamHandle->client()->didClose(m_streamHandle);
>
>Since this is called in case of socket errors, shouldn't we call didFail() on
>the stream handle client instead of just didClose()?
Chromium port is calling didClose(), so I followed it, but I will look into this again

>> +void SocketStreamHandlePrivate::socketSslErrors(const QList<QSslError>&)
>> +{
>> +    qobject_cast<QSslSocket*>(m_socket)->ignoreSslErrors();
>> +}
>
>Hmmm, doesn't that effectively remove any security from encrypted connections?
>If someone spoofs a certificate we ignore it and still happily connect,
>allowing a man-in-the-middle to read all the traffic.
Yes, I acknowledged this in comment #2, as a missing feature.


>>  # Missing SocketStreamHandle implementation
>> -websocket/tests>
>
>I like this part! Yay for layout testing :)
>
>But you should also remove the comment then that says the implementation is
>missing ;-)
oops!

[Yael]

Changing didClose() to didFail() caused 7 layout tests to fail. Since the error code is not used for anything, I would like to keep the didClose() call.

[Yael]

Created attachment 47547 [details]
Patch v3

Thanks to valgrind I found and fixed the random crashes. 
With this patch proxy authentication still does not work, and for the purpose of testing, I allow wss connections with bad certificates. This should be disabled later on.

[Laszlo Gombos]

v3 looks good to me - with the minor comment to include the status of the implementation in the ChangeLog as well.

[Yael]

Created attachment 47551 [details]
Patch v4

Added more comments at Laszlo's request.

[Laszlo Gombos]

Comment on attachment 47551 [details]
Patch v4

Looks good to me; all the previous review comments have been addressed.

[WebKit Commit Bot]

Comment on attachment 47551 [details]
Patch v4

Clearing flags on attachment: 47551

Committed r53947: <http://trac.webkit.org/changeset/53947>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Markus Goetz]

Small note: Theoretically, the SSL channel can only be seen as open after encrypted() has been emitted. The connected() signal is for the underlying TCP connection.

I don't know if it is worth to fix this or not.

[Markus Goetz]

The check "m_socket->state() == QAbstractSocket::ConnectedState" should maybe be removed since we also want to close a socket that is e.g. still in HostLookupState.

[Yael]

Created https://bugs.webkit.org/show_bug.cgi?id=34425 to address comments #14 and #15.