Changeset 249158 in webkit


Ignore:
Timestamp:
Aug 27, 2019 11:59:29 AM (5 years ago)
Author:
Basuke Suzuki
Message:

[RemoteInspector][Socket] Restructuring the components of Socket implementation
​https://bugs.webkit.org/show_bug.cgi?id=201079

Reviewed by Ross Kirsling.

Source/JavaScriptCore:

Since the change for WeakPtr on r248386, our port start assertion failure on the usage of
RemoteInspectorSocketEndpoint. We have to send a message to connection client, but if that
has to be done in the same thread which weakPtr generated, it's a little bit stronger
restriction for us to handle. In this restructure, we are stopping to use weakPtr to
resolve circular dependency, but using a reference with invalidation method because
everything is under our control.

  • Make SocketEndpoint a singleton. This class represents a central place to handle socket connections and there's no need to instantiate more than one in a process. Once every connection goes away, it just start sleeping until next connection is created. Very low resource usage when it is idle.
  • Move Socket::Connection structure from global definition to SocketEndpoint local structure. It is directly used in SocketEndpoint privately.
  • Move responsibility to handle message encoding/decoding task from SocketEndpoint to ConnectionClient. Make SocketEndpoint as plain socket handling as possible to keep it simple to exist long span.
  • Extract an interface from ConnectionClient as SocketEndpoint::Client which is required to work with SocketEndpoint. Now SocketEndpoint is very independent from others. SocketEndpoint::Client is the required parameter to create a connection.

Many responsibilities are moved into ConnectionClient which was a thin interface for
communication between RemoteInspector, RemoteInspectorServer and RemoteInspectorClient.
It now handles followings:

  • life cycle of connection: create, listen and close or invalidation
  • sending and receiving data packed in a message.

RemoteInspector and RemoteInspectorServer are now free from creation of SocketEndpoint.
All communication to SocketEndpoint id now the duty of super class.

  • inspector/remote/RemoteInspector.h:
  • inspector/remote/socket/RemoteInspectorConnectionClient.cpp:

(Inspector::RemoteInspectorConnectionClient::~RemoteInspectorConnectionClient): Make all connection invalidated.
(Inspector::RemoteInspectorConnectionClient::connectInet): Add itself as a listener of socket.
(Inspector::RemoteInspectorConnectionClient::listenInet): Ditto.
(Inspector::RemoteInspectorConnectionClient::createClient): Ditto.
(Inspector::RemoteInspectorConnectionClient::send): Add message processing.
(Inspector::RemoteInspectorConnectionClient::didReceive): Ditto.
(Inspector::RemoteInspectorConnectionClient::extractEvent): Extracted from send.

  • inspector/remote/socket/RemoteInspectorConnectionClient.h:
  • inspector/remote/socket/RemoteInspectorMessageParser.cpp:

(Inspector::MessageParser::MessageParser):
(Inspector::MessageParser::pushReceivedData):
(Inspector::MessageParser::parse):

  • inspector/remote/socket/RemoteInspectorMessageParser.h:

(Inspector::MessageParser::MessageParser):
(Inspector::MessageParser::Function<void):

  • inspector/remote/socket/RemoteInspectorServer.cpp:

(Inspector::RemoteInspectorServer::connect): Remove direct communication to Socket Endpoint.
(Inspector::RemoteInspectorServer::listenForTargets): Ditto.
(Inspector::RemoteInspectorServer::sendWebInspectorEvent): Ditto.
(Inspector::RemoteInspectorServer::start): Ditto.

  • inspector/remote/socket/RemoteInspectorServer.h:
  • inspector/remote/socket/RemoteInspectorSocket.cpp:

(Inspector::RemoteInspector::sendWebInspectorEvent): Remove direct communication to Socket Endpoint.
(Inspector::RemoteInspector::start): Ditto.
(Inspector::RemoteInspector::stopInternal): Ditto.
(Inspector::RemoteInspector::pushListingsNow): Change the target of validity check to ID.
(Inspector::RemoteInspector::pushListingsSoon): Ditto.
(Inspector::RemoteInspector::sendMessageToRemote): Ditto.

  • inspector/remote/socket/RemoteInspectorSocket.h: Move Connection structure to RemoteInspectorSocketEndpoint.
  • inspector/remote/socket/RemoteInspectorSocketEndpoint.cpp:

(Inspector::RemoteInspectorSocketEndpoint::singleton): Added.
(Inspector::RemoteInspectorSocketEndpoint::RemoteInspectorSocketEndpoint): Use hard-coded thread name.
(Inspector::RemoteInspectorSocketEndpoint::connectInet): Accept RemoteInspectorSocketEndpoint::Client as listener.
(Inspector::RemoteInspectorSocketEndpoint::listenInet): Ditto.
(Inspector::RemoteInspectorSocketEndpoint::createClient): Ditto.
(Inspector::RemoteInspectorSocketEndpoint::invalidateClient): Added. Invalidate all connection from the client.
(Inspector::RemoteInspectorSocketEndpoint::recvIfEnabled): Remove message parser handling.
(Inspector::RemoteInspectorSocketEndpoint::send): Remove message packing.
(Inspector::RemoteInspectorSocketEndpoint::acceptInetSocketIfEnabled):

  • inspector/remote/socket/RemoteInspectorSocketEndpoint.h:

(Inspector::RemoteInspectorSocketEndpoint::Connection::Connection):

Source/WebKit:

RemoteInspectorClient is now free from creation of SocketEndpoint. All communication
to SocketEndpoint id now the duty of super class.

  • UIProcess/socket/RemoteInspectorClient.cpp:

(WebKit::RemoteInspectorClient::RemoteInspectorClient): Remove direct communication to Socket Endpoint.
(WebKit::RemoteInspectorClient::sendWebInspectorEvent): Ditto.

  • UIProcess/socket/RemoteInspectorClient.h:
Location:
trunk/Source
Files:
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r249132 r249158  
     12019-08-27  Basuke Suzuki  <Basuke.Suzuki@sony.com>
     2
     3        [RemoteInspector][Socket] Restructuring the components of Socket implementation
     4        https://bugs.webkit.org/show_bug.cgi?id=201079
     5
     6        Reviewed by Ross Kirsling.
     7
     8        Since the change for WeakPtr on r248386, our port start assertion failure on the usage of
     9        RemoteInspectorSocketEndpoint. We have to send a message to connection client, but if that
     10        has to be done in the same thread which weakPtr generated, it's a little bit stronger
     11        restriction for us to handle. In this restructure, we are stopping to use weakPtr to
     12        resolve circular dependency, but using a reference with invalidation method because
     13        everything is under our control.
     14
     15        - Make SocketEndpoint a singleton. This class represents a central place to handle socket
     16          connections and there's no need to instantiate more than one in a process. Once every
     17          connection goes away, it just start sleeping until next connection is created. Very low
     18          resource usage when it is idle.
     19        - Move Socket::Connection structure from global definition to SocketEndpoint local
     20          structure. It is directly used in SocketEndpoint privately.
     21        - Move responsibility to handle message encoding/decoding task from SocketEndpoint to
     22          ConnectionClient. Make SocketEndpoint as plain socket handling as possible to keep it
     23          simple to exist long span.
     24        - Extract an interface from ConnectionClient as SocketEndpoint::Client which is required
     25          to work with SocketEndpoint. Now SocketEndpoint is very independent from others.
     26          SocketEndpoint::Client is the required parameter to create a connection.
     27
     28        Many responsibilities are moved into ConnectionClient which was a thin interface for
     29        communication between RemoteInspector, RemoteInspectorServer and RemoteInspectorClient.
     30        It now handles followings:
     31        - life cycle of connection: create, listen and close or invalidation
     32        - sending and receiving data packed in a message.
     33
     34        RemoteInspector and RemoteInspectorServer are now free from creation of SocketEndpoint.
     35        All communication to SocketEndpoint id now the duty of super class.
     36
     37        * inspector/remote/RemoteInspector.h:
     38        * inspector/remote/socket/RemoteInspectorConnectionClient.cpp:
     39        (Inspector::RemoteInspectorConnectionClient::~RemoteInspectorConnectionClient): Make all connection invalidated.
     40        (Inspector::RemoteInspectorConnectionClient::connectInet): Add itself as a listener of socket.
     41        (Inspector::RemoteInspectorConnectionClient::listenInet): Ditto.
     42        (Inspector::RemoteInspectorConnectionClient::createClient): Ditto.
     43        (Inspector::RemoteInspectorConnectionClient::send): Add message processing.
     44        (Inspector::RemoteInspectorConnectionClient::didReceive): Ditto.
     45        (Inspector::RemoteInspectorConnectionClient::extractEvent): Extracted from send.
     46        * inspector/remote/socket/RemoteInspectorConnectionClient.h:
     47        * inspector/remote/socket/RemoteInspectorMessageParser.cpp:
     48        (Inspector::MessageParser::MessageParser):
     49        (Inspector::MessageParser::pushReceivedData):
     50        (Inspector::MessageParser::parse):
     51        * inspector/remote/socket/RemoteInspectorMessageParser.h:
     52        (Inspector::MessageParser::MessageParser):
     53        (Inspector::MessageParser::Function<void):
     54        * inspector/remote/socket/RemoteInspectorServer.cpp:
     55        (Inspector::RemoteInspectorServer::connect): Remove direct communication to Socket Endpoint.
     56        (Inspector::RemoteInspectorServer::listenForTargets): Ditto.
     57        (Inspector::RemoteInspectorServer::sendWebInspectorEvent): Ditto.
     58        (Inspector::RemoteInspectorServer::start): Ditto.
     59        * inspector/remote/socket/RemoteInspectorServer.h:
     60        * inspector/remote/socket/RemoteInspectorSocket.cpp:
     61        (Inspector::RemoteInspector::sendWebInspectorEvent): Remove direct communication to Socket Endpoint.
     62        (Inspector::RemoteInspector::start): Ditto.
     63        (Inspector::RemoteInspector::stopInternal): Ditto.
     64        (Inspector::RemoteInspector::pushListingsNow): Change the target of validity check to ID.
     65        (Inspector::RemoteInspector::pushListingsSoon): Ditto.
     66        (Inspector::RemoteInspector::sendMessageToRemote): Ditto.
     67        * inspector/remote/socket/RemoteInspectorSocket.h: Move Connection structure to RemoteInspectorSocketEndpoint.
     68        * inspector/remote/socket/RemoteInspectorSocketEndpoint.cpp:
     69        (Inspector::RemoteInspectorSocketEndpoint::singleton): Added.
     70        (Inspector::RemoteInspectorSocketEndpoint::RemoteInspectorSocketEndpoint): Use hard-coded thread name.
     71        (Inspector::RemoteInspectorSocketEndpoint::connectInet): Accept RemoteInspectorSocketEndpoint::Client as listener.
     72        (Inspector::RemoteInspectorSocketEndpoint::listenInet): Ditto.
     73        (Inspector::RemoteInspectorSocketEndpoint::createClient): Ditto.
     74        (Inspector::RemoteInspectorSocketEndpoint::invalidateClient): Added. Invalidate all connection from the client.
     75        (Inspector::RemoteInspectorSocketEndpoint::recvIfEnabled): Remove message parser handling.
     76        (Inspector::RemoteInspectorSocketEndpoint::send): Remove message packing.
     77        (Inspector::RemoteInspectorSocketEndpoint::acceptInetSocketIfEnabled):
     78        * inspector/remote/socket/RemoteInspectorSocketEndpoint.h:
     79        (Inspector::RemoteInspectorSocketEndpoint::Connection::Connection):
     80
    1812019-08-26  Devin Rousso  <drousso@apple.com>
    282

  • trunk/Source/JavaScriptCore/inspector/remote/RemoteInspector.h

    r244919 r249158  
    5757#include "RemoteConnectionToTarget.h"
    5858#include "RemoteInspectorConnectionClient.h"
    59 #include "RemoteInspectorSocketEndpoint.h"
    6059#include <wtf/JSONValues.h>
    6160#include <wtf/RefCounted.h>
    … …  
    242241
    243242#if USE(INSPECTOR_SOCKET_SERVER)
    244     std::unique_ptr<RemoteInspectorSocketEndpoint> m_socketConnection;
    245243    static PlatformSocketType s_connectionIdentifier;
    246244    static uint16_t s_serverPort;

  • trunk/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorConnectionClient.cpp

    r245536 r249158  
    3535namespace Inspector {
    3636
    37 void RemoteInspectorConnectionClient::didReceiveWebInspectorEvent(ConnectionID clientID, Vector<uint8_t>&& data)
     37RemoteInspectorConnectionClient::~RemoteInspectorConnectionClient()
     38{
     39    auto& endpoint = Inspector::RemoteInspectorSocketEndpoint::singleton();
     40    endpoint.invalidateClient(*this);
     41}
     42
     43Optional<ConnectionID> RemoteInspectorConnectionClient::connectInet(const char* serverAddr, uint16_t serverPort)
     44{
     45    auto& endpoint = Inspector::RemoteInspectorSocketEndpoint::singleton();
     46    return endpoint.connectInet(serverAddr, serverPort, *this);
     47}
     48
     49Optional<ConnectionID> RemoteInspectorConnectionClient::listenInet(const char* address, uint16_t port)
     50{
     51    auto& endpoint = Inspector::RemoteInspectorSocketEndpoint::singleton();
     52    return endpoint.listenInet(address, port, *this);
     53}
     54
     55Optional<ConnectionID> RemoteInspectorConnectionClient::createClient(PlatformSocketType socket)
     56{
     57    auto& endpoint = Inspector::RemoteInspectorSocketEndpoint::singleton();
     58    return endpoint.createClient(socket, *this);
     59}
     60
     61void RemoteInspectorConnectionClient::send(ConnectionID id, const uint8_t* data, size_t size)
     62{
     63    auto message = MessageParser::createMessage(data, size);
     64    if (message.isEmpty())
     65        return;
     66
     67    auto& endpoint = RemoteInspectorSocketEndpoint::singleton();
     68    endpoint.send(id, message.data(), message.size());
     69}
     70
     71void RemoteInspectorConnectionClient::didReceive(ConnectionID clientID, Vector<uint8_t>&& data)
    3872{
    3973    ASSERT(!isMainThread());
    4074
     75    LockHolder lock(m_parsersLock);
     76    auto result = m_parsers.ensure(clientID, [this, clientID] {
     77        return MessageParser([this, clientID](Vector<uint8_t>&& data) {
     78            if (auto event = RemoteInspectorConnectionClient::extractEvent(clientID, WTFMove(data))) {
     79                RunLoop::main().dispatch([this, event = WTFMove(*event)] {
     80                    const auto& methodName = event.methodName;
     81                    auto& methods = dispatchMap();
     82                    if (methods.contains(methodName)) {
     83                        auto call = methods.get(methodName);
     84                        (this->*call)(event);
     85                    } else
     86                        LOG_ERROR("Unknown event: %s", methodName.utf8().data());
     87                });
     88            }
     89        });
     90    });
     91    result.iterator->value.pushReceivedData(data.data(), data.size());
     92}
     93
     94Optional<RemoteInspectorConnectionClient::Event> RemoteInspectorConnectionClient::extractEvent(ConnectionID clientID, Vector<uint8_t>&& data)
     95{
    4196    if (data.isEmpty())
    42         return;
     97        return WTF::nullopt;
    4398
    4499    String jsonData = String::fromUTF8(data);
    … …  
    46101    RefPtr<JSON::Value> messageValue;
    47102    if (!JSON::Value::parseJSON(jsonData, messageValue))
    48         return;
     103        return WTF::nullopt;
    49104
    50105    RefPtr<JSON::Object> messageObject;
    51106    if (!messageValue->asObject(messageObject))
    52         return;
    53 
    54     String methodName;
    55     if (!messageObject->getString("event"_s, methodName))
    56         return;
     107        return WTF::nullopt;
    57108
    58109    Event event;
     110    if (!messageObject->getString("event"_s, event.methodName))
     111        return WTF::nullopt;
     112
    59113    event.clientID = clientID;
    60114
    … …  
    71125        event.message = message;
    72126
    73     RunLoop::main().dispatch([this, methodName, event = WTFMove(event)] {
    74         auto& methods = dispatchMap();
    75         if (methods.contains(methodName)) {
    76             auto call = methods.get(methodName);
    77             (this->*call)(event);
    78         } else
    79             LOG_ERROR("Unknown event: %s", methodName.utf8().data());
    80     });
     127    return event;
    81128}
    82129

  • trunk/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorConnectionClient.h

    r246299 r249158  
    2929
    3030#include "RemoteControllableTarget.h"
     31#include "RemoteInspectorMessageParser.h"
    3132#include "RemoteInspectorSocketEndpoint.h"
    32 #include <wtf/WeakPtr.h>
     33#include <wtf/HashMap.h>
     34#include <wtf/Lock.h>
    3335#include <wtf/text/WTFString.h>
    3436
    3537namespace Inspector {
    3638
    37 class RemoteInspectorConnectionClient : public CanMakeWeakPtr<RemoteInspectorConnectionClient> {
     39class MessageParser;
     40
     41class JS_EXPORT_PRIVATE RemoteInspectorConnectionClient : public RemoteInspectorSocketEndpoint::Client {
    3842public:
    39     void didReceiveWebInspectorEvent(ConnectionID, Vector<uint8_t>&&);
    40     virtual void didAccept(ConnectionID /* acceptedID */, ConnectionID /* listenerID */, Socket::Domain) { }
    41     virtual void didClose(ConnectionID) = 0;
     43    virtual ~RemoteInspectorConnectionClient();
     44
     45    Optional<ConnectionID> connectInet(const char* serverAddr, uint16_t serverPort);
     46    Optional<ConnectionID> listenInet(const char* address, uint16_t port);
     47    Optional<ConnectionID> createClient(PlatformSocketType);
     48    void send(ConnectionID, const uint8_t* data, size_t);
     49
     50    void didReceive(ConnectionID, Vector<uint8_t>&&) override;
     51    void didAccept(ConnectionID, ConnectionID, Socket::Domain) override { }
    4252
    4353    struct Event {
     54        String methodName;
    4455        ConnectionID clientID { };
    4556        Optional<ConnectionID> connectionID;
    … …  
    5061    using CallHandler = void (RemoteInspectorConnectionClient::*)(const Event&);
    5162    virtual HashMap<String, CallHandler>& dispatchMap() = 0;
     63
     64protected:
     65    static Optional<Event> extractEvent(ConnectionID, Vector<uint8_t>&&);
     66
     67    HashMap<ConnectionID, MessageParser> m_parsers;
     68    Lock m_parsersLock;
    5269};
    5370

  • trunk/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorMessageParser.cpp

    r244657 r249158  
    4242*/
    4343
    44 MessageParser::MessageParser(ConnectionID id, size_t bufferSize)
    45     : m_connectionID(id)
     44MessageParser::MessageParser(Function<void(Vector<uint8_t>&&)>&& listener)
     45    : m_listener(WTFMove(listener))
    4646{
    47     m_buffer.reserveCapacity(bufferSize);
    4847}
    4948
    … …  
    6362void MessageParser::pushReceivedData(const uint8_t* data, size_t size)
    6463{
    65     if (!data || !size)
     64    if (!data || !size || !m_listener)
    6665        return;
    6766
    … …  
    104103        memcpy(&dataBuffer[0], &m_buffer[sizeof(uint32_t)], dataSize);
    105104
    106         if (m_didParseMessageListener)
    107             m_didParseMessageListener(m_connectionID, WTFMove(dataBuffer));
     105        m_listener(WTFMove(dataBuffer));
    108106
    109107        m_buffer.remove(0, messageSize);

  • trunk/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorMessageParser.h

    r248546 r249158  
    2828#if ENABLE(REMOTE_INSPECTOR)
    2929
    30 #include "RemoteInspectorSocketEndpoint.h"
     30#include <wtf/Function.h>
    3131#include <wtf/Vector.h>
    3232
    … …  
    3838    static Vector<uint8_t> createMessage(const uint8_t*, size_t);
    3939
    40     MessageParser(ConnectionID, size_t);
     40    MessageParser() { }
     41    MessageParser(Function<void(Vector<uint8_t>&&)>&&);
    4142    void pushReceivedData(const uint8_t*, size_t);
    42     void setDidParseMessageListener(Function<void(ConnectionID, Vector<uint8_t>)>&& listener) { m_didParseMessageListener = WTFMove(listener); }
    43 
    4443    void clearReceivedData();
    4544
    … …  
    4746    bool parse();
    4847
    49     Function<void(ConnectionID, Vector<uint8_t>&&)> m_didParseMessageListener;
     48    Function<void(Vector<uint8_t>&&)> m_listener { };
    5049    Vector<uint8_t> m_buffer;
    51     ConnectionID m_connectionID;
    5250};
    5351

  • trunk/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorServer.cpp

    r246860 r249158  
    2929#if ENABLE(REMOTE_INSPECTOR)
    3030
     31#include "RemoteInspectorMessageParser.h"
     32
    3133#include <wtf/JSONValues.h>
    3234#include <wtf/MainThread.h>
    … …  
    3638Optional<PlatformSocketType> RemoteInspectorServer::connect()
    3739{
    38     if (!m_server) {
    39         LOG_ERROR("Inspector server is not running");
    40         return WTF::nullopt;
    41     }
    42 
    4340    if (auto sockets = Socket::createPair()) {
    44         if (auto id = m_server->createClient(sockets->at(0))) {
     41        if (auto id = createClient(sockets->at(0))) {
    4542            LockHolder lock(m_connectionsLock);
    4643            m_inspectorConnections.append(id.value());
    … …  
    5552Optional<uint16_t> RemoteInspectorServer::listenForTargets()
    5653{
    57     if (!m_server) {
    58         LOG_ERROR("Inspector server is not running");
    59         return WTF::nullopt;
    60     }
    61 
    6254    if (m_inspectorListener) {
    6355        LOG_ERROR("Inspector server is already listening for targets.");
    … …  
    6557    }
    6658
    67     if (auto connection = m_server->listenInet("127.0.0.1", 0)) {
     59    if (auto connection = listenInet("127.0.0.1", 0)) {
    6860        m_inspectorListener = connection;
    69         return m_server->getPort(*connection);
     61
     62        auto& endpoint = RemoteInspectorSocketEndpoint::singleton();
     63        return endpoint.getPort(*connection);
    7064    }
    7165
    … …  
    124118{
    125119    const CString message = event.utf8();
    126     m_server->send(id, reinterpret_cast<const uint8_t*>(message.data()), message.length());
     120    send(id, reinterpret_cast<const uint8_t*>(message.data()), message.length());
    127121}
    128122
    … …  
    135129bool RemoteInspectorServer::start(const char* address, uint16_t port)
    136130{
    137     m_server = RemoteInspectorSocketEndpoint::create(this, "RemoteInspectorServer");
    138 
    139     if (!m_server->listenInet(address, port)) {
    140         m_server = nullptr;
    141         return false;
    142     }
    143 
    144     return true;
     131    m_server = listenInet(address, port);
     132    return isRunning();
    145133}
    146134

  • trunk/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorServer.h

    r246860 r249158  
    7171
    7272    HashSet<std::pair<ConnectionID, TargetID>> m_inspectionTargets;
    73     std::unique_ptr<RemoteInspectorSocketEndpoint> m_server;
     73
     74    Optional<ConnectionID> m_server;
    7475
    7576    // Connections to the WebProcess.
    … …  
    8081    Optional<ConnectionID> m_inspectorListener;
    8182
    82     // Connections from RemoteInspectorClient.
     83    // Connection from RemoteInspectorClient.
    8384    Optional<ConnectionID> m_clientConnection;
    8485};

  • trunk/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp

    r245726 r249158  
    8282
    8383    const CString message = event.utf8();
    84     m_socketConnection->send(m_clientID.value(), reinterpret_cast<const uint8_t*>(message.data()), message.length());
     84    send(m_clientID.value(), reinterpret_cast<const uint8_t*>(message.data()), message.length());
    8585}
    8686
    … …  
    9494    m_enabled = true;
    9595
    96     m_socketConnection = RemoteInspectorSocketEndpoint::create(this, "RemoteInspector");
    97 
    9896    if (s_connectionIdentifier) {
    99         m_clientID = m_socketConnection->createClient(s_connectionIdentifier);
     97        m_clientID = createClient(s_connectionIdentifier);
    10098        s_connectionIdentifier = INVALID_SOCKET_VALUE;
    10199    } else
    102         m_clientID = m_socketConnection->connectInet("127.0.0.1", s_serverPort);
     100        m_clientID = connectInet("127.0.0.1", s_serverPort);
    103101
    104102    if (!m_targetMap.isEmpty())
    … …  
    121119
    122120    m_automaticInspectionPaused = false;
    123     m_socketConnection = nullptr;
    124121    m_clientID = WTF::nullopt;
    125122}
    … …  
    157154void RemoteInspector::pushListingsNow()
    158155{
    159     if (!m_socketConnection)
     156    if (!m_clientID)
    160157        return;
    161158
    … …  
    174171void RemoteInspector::pushListingsSoon()
    175172{
    176     if (!m_socketConnection)
     173    if (!m_clientID)
    177174        return;
    178175
    … …  
    196193{
    197194    LockHolder lock(m_mutex);
    198     if (!m_socketConnection)
     195    if (!m_clientID)
    199196        return;
    200197

  • trunk/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.h

    r248546 r249158  
    3232#include <wtf/Vector.h>
    3333
    34 #if PLATFORM(PLAYSTATION)
    35 #include <poll.h>
    36 #endif
    37 
    3834#if OS(WINDOWS)
    3935#include <winsock2.h>
     36#else
     37#include <poll.h>
    4038#endif
    4139
    … …  
    5755
    5856#endif
    59 
    60 class MessageParser;
    6157
    6258namespace Socket {
    … …  
    9187void clearWaitingWritable(PollingDescriptor&);
    9288
    93 struct Connection {
    94     WTF_MAKE_STRUCT_FAST_ALLOCATED;
    95     std::unique_ptr<MessageParser> parser;
    96     Vector<uint8_t> sendBuffer;
    97     PlatformSocketType socket { INVALID_SOCKET_VALUE };
    98     PollingDescriptor poll;
    99 };
    100 
    10189constexpr size_t BufferSize = 65536;
    10290

  • trunk/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocketEndpoint.cpp

    r248846 r249158  
    2929#if ENABLE(REMOTE_INSPECTOR)
    3030
    31 #include "RemoteInspectorConnectionClient.h"
    32 #include "RemoteInspectorMessageParser.h"
    3331#include <wtf/CryptographicallyRandomNumber.h>
    3432#include <wtf/MainThread.h>
    … …  
    3735namespace Inspector {
    3836
    39 RemoteInspectorSocketEndpoint::RemoteInspectorSocketEndpoint(RemoteInspectorConnectionClient* inspectorClient, const char* name)
    40     : m_inspectorClient(makeWeakPtr(inspectorClient))
     37RemoteInspectorSocketEndpoint& RemoteInspectorSocketEndpoint::singleton()
     38{
     39    static NeverDestroyed<RemoteInspectorSocketEndpoint> shared;
     40    return shared;
     41}
     42
     43RemoteInspectorSocketEndpoint::RemoteInspectorSocketEndpoint()
    4144{
    4245    if (auto sockets = Socket::createPair()) {
    … …  
    4548    }
    4649
    47     m_workerThread = Thread::create(name, [this] {
     50    m_workerThread = Thread::create("SocketEndpoint", [this] {
    4851        workerThread();
    4952    });
    … …  
    7073}
    7174
    72 Optional<ConnectionID> RemoteInspectorSocketEndpoint::connectInet(const char* serverAddress, uint16_t serverPort)
     75Optional<ConnectionID> RemoteInspectorSocketEndpoint::connectInet(const char* serverAddress, uint16_t serverPort, Client& client)
    7376{
    7477    if (auto socket = Socket::connect(serverAddress, serverPort))
    75         return createClient(*socket);
     78        return createClient(*socket, client);
    7679    return WTF::nullopt;
    7780}
    7881
    79 Optional<ConnectionID> RemoteInspectorSocketEndpoint::listenInet(const char* address, uint16_t port)
     82Optional<ConnectionID> RemoteInspectorSocketEndpoint::listenInet(const char* address, uint16_t port, Client& client)
    8083{
    8184    if (auto socket = Socket::listen(address, port))
    82         return createClient(*socket);
     85        return createClient(*socket, client);
    8386
    8487    return WTF::nullopt;
    … …  
    131134}
    132135
    133 Optional<ConnectionID> RemoteInspectorSocketEndpoint::createClient(PlatformSocketType socket)
     136Optional<ConnectionID> RemoteInspectorSocketEndpoint::createClient(PlatformSocketType socket, Client& client)
    134137{
    135138    if (!Socket::isValid(socket))
    … …  
    145148    Socket::setup(socket);
    146149
    147     auto connection = makeUnique<Socket::Connection>();
    148 
     150    auto connection = makeUnique<Connection>(client);
     151
     152    connection->id = id;
    149153    connection->poll = Socket::preparePolling(socket);
    150154    connection->socket = socket;
    151     connection->parser = makeUnique<MessageParser>(id, Socket::BufferSize);
    152     connection->parser->setDidParseMessageListener([this](ConnectionID id, Vector<uint8_t>&& data) {
    153         if (m_inspectorClient)
    154             m_inspectorClient->didReceiveWebInspectorEvent(id, WTFMove(data));
    155     });
    156155    m_connections.add(id, WTFMove(connection));
    157156    wakeupWorkerThread();
    … …  
    160159}
    161160
     161void RemoteInspectorSocketEndpoint::invalidateClient(Client& client)
     162{
     163    LockHolder lock(m_connectionsLock);
     164    m_connections.removeIf([&client](auto& keyValue) {
     165        const auto& connection = keyValue.value;
     166
     167        if (&connection->client != &client)
     168            return false;
     169
     170        Socket::close(connection->socket);
     171        // do not call client.didClose because client is already invalidating phase.
     172        return true;
     173    });
     174}
     175
    162176Optional<uint16_t> RemoteInspectorSocketEndpoint::getPort(ConnectionID id) const
    163177{
    … …  
    175189        Vector<uint8_t> recvBuffer(Socket::BufferSize);
    176190        if (auto readSize = Socket::read(connection->socket, recvBuffer.data(), recvBuffer.size())) {
    177             if (*readSize > 0)
    178                 connection->parser->pushReceivedData(recvBuffer.data(), *readSize);
    179             return;
     191            if (*readSize > 0) {
     192                recvBuffer.shrink(*readSize);
     193                connection->client.didReceive(id, WTFMove(recvBuffer));
     194                return;
     195            }
    180196        }
    181197
    … …  
    184200
    185201        lock.unlockEarly();
    186         if (m_inspectorClient)
    187             m_inspectorClient->didClose(id);
     202        connection->client.didClose(id);
    188203    }
    189204}
    … …  
    218233    LockHolder lock(m_connectionsLock);
    219234    if (const auto& connection = m_connections.get(id)) {
    220         auto message = MessageParser::createMessage(data, size);
    221         if (message.isEmpty())
    222             return;
    223 
    224235        size_t offset = 0;
    225236        if (connection->sendBuffer.isEmpty()) {
    226237            // Try to call send() directly if buffer is empty.
    227             if (auto writeSize = Socket::write(connection->socket, message.data(), std::min(message.size(), Socket::BufferSize)))
     238            if (auto writeSize = Socket::write(connection->socket, data, std::min(size, Socket::BufferSize)))
    228239                offset = *writeSize;
    229240            // @TODO need to handle closed socket case?
    … …  
    231242
    232243        // Check all data is sent.
    233         if (offset == message.size())
     244        if (offset == size)
    234245            return;
    235246
    236247        // Copy remaining data to send later.
    237         connection->sendBuffer.appendRange(message.begin() + offset, message.end());
     248        connection->sendBuffer.appendRange(data + offset, data + size);
    238249        Socket::markWaitingWritable(connection->poll);
    239250
    … …  
    252263            // Need to unlock before calling createClient as it also attempts to lock.
    253264            lock.unlockEarly();
    254             if (auto newID = createClient(*socket)) {
    255                 if (m_inspectorClient) {
    256                     m_inspectorClient->didAccept(newID.value(), id, Socket::Domain::Network);
    257                     return;
    258                 }
     265            if (auto newID = createClient(*socket, connection->client)) {
     266                connection->client.didAccept(newID.value(), id, Socket::Domain::Network);
     267                return;
    259268            }
    260269

  • trunk/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocketEndpoint.h

    r248846 r249158  
    3636#include <wtf/Threading.h>
    3737#include <wtf/Vector.h>
    38 #include <wtf/WeakPtr.h>
    3938
    4039namespace Inspector {
    4140
    42 class MessageParser;
    43 class RemoteInspectorConnectionClient;
    44 
    45 class JS_EXPORT_PRIVATE RemoteInspectorSocketEndpoint {
     41class RemoteInspectorSocketEndpoint {
    4642    WTF_MAKE_FAST_ALLOCATED;
    4743public:
    48     static std::unique_ptr<RemoteInspectorSocketEndpoint> create(RemoteInspectorConnectionClient* inspectorClient, const char* name)
    49     {
    50         return makeUnique<RemoteInspectorSocketEndpoint>(inspectorClient, name);
    51     }
     44    class Client {
     45    public:
     46        virtual void didReceive(ConnectionID, Vector<uint8_t>&&) = 0;
     47        virtual void didAccept(ConnectionID acceptedID, ConnectionID listenerID, Socket::Domain) = 0;
     48        virtual void didClose(ConnectionID) = 0;
     49    };
    5250
    53     RemoteInspectorSocketEndpoint(RemoteInspectorConnectionClient*, const char*);
     51    static RemoteInspectorSocketEndpoint& singleton();
     52
     53    RemoteInspectorSocketEndpoint();
    5454    ~RemoteInspectorSocketEndpoint();
    5555
    56     Optional<ConnectionID> connectInet(const char* serverAddr, uint16_t serverPort);
    57     Optional<ConnectionID> listenInet(const char* address, uint16_t port);
     56    Optional<ConnectionID> connectInet(const char* serverAddr, uint16_t serverPort, Client&);
     57    Optional<ConnectionID> listenInet(const char* address, uint16_t port, Client&);
     58    void invalidateClient(Client&);
    5859
    5960    void send(ConnectionID, const uint8_t* data, size_t);
    6061
    61     Optional<ConnectionID> createClient(PlatformSocketType fd);
     62    Optional<ConnectionID> createClient(PlatformSocketType, Client&);
    6263
    6364    Optional<uint16_t> getPort(ConnectionID) const;
    6465
    6566protected:
     67    struct Connection {
     68        WTF_MAKE_STRUCT_FAST_ALLOCATED;
     69        explicit Connection(Client& client)
     70            : client(client)
     71        {
     72        }
     73
     74        ConnectionID id;
     75        Vector<uint8_t> sendBuffer;
     76        PlatformSocketType socket { INVALID_SOCKET_VALUE };
     77        PollingDescriptor poll;
     78        Client& client;
     79    };
     80
    6681    void recvIfEnabled(ConnectionID);
    6782    void sendIfEnabled(ConnectionID);
    … …  
    7287
    7388    mutable Lock m_connectionsLock;
    74     HashMap<ConnectionID, std::unique_ptr<Socket::Connection>> m_connections;
     89    HashMap<ConnectionID, std::unique_ptr<Connection>> m_connections;
    7590
    7691    PlatformSocketType m_wakeupSendSocket { INVALID_SOCKET_VALUE };
    … …  
    7994    RefPtr<Thread> m_workerThread;
    8095    std::atomic<bool> m_shouldAbortWorkerThread { false };
    81 
    82     WeakPtr<RemoteInspectorConnectionClient> m_inspectorClient;
    8396};
    8497

  • trunk/Source/WebKit/ChangeLog

    r249155 r249158  
     12019-08-27  Basuke Suzuki  <Basuke.Suzuki@sony.com>
     2
     3        [RemoteInspector][Socket] Restructuring the components of Socket implementation
     4        https://bugs.webkit.org/show_bug.cgi?id=201079
     5
     6        Reviewed by Ross Kirsling.
     7
     8        RemoteInspectorClient is now free from creation of SocketEndpoint. All communication
     9        to SocketEndpoint id now the duty of super class.
     10
     11        * UIProcess/socket/RemoteInspectorClient.cpp:
     12        (WebKit::RemoteInspectorClient::RemoteInspectorClient): Remove direct communication to Socket Endpoint.
     13        (WebKit::RemoteInspectorClient::sendWebInspectorEvent): Ditto.
     14        * UIProcess/socket/RemoteInspectorClient.h:
     15
    1162019-08-27  Chris Dumez  <cdumez@apple.com>
    217

  • trunk/Source/WebKit/UIProcess/socket/RemoteInspectorClient.cpp

    r248846 r249158  
    8989    : m_observer(observer)
    9090{
    91     m_socket = Inspector::RemoteInspectorSocketEndpoint::create(this, "RemoteInspectorClient");
    92 
    93     m_connectionID = m_socket->connectInet(address, port);
     91    m_connectionID = connectInet(address, port);
    9492    if (!m_connectionID) {
    9593        LOG_ERROR("Inspector client could not connect to %s:%d", address, port);
    96         m_socket = nullptr;
    9794        return;
    9895    }
    … …  
    110107    ASSERT(m_connectionID.hasValue());
    111108    auto message = event.utf8();
    112     m_socket->send(m_connectionID.value(), reinterpret_cast<const uint8_t*>(message.data()), message.length());
     109    send(m_connectionID.value(), reinterpret_cast<const uint8_t*>(message.data()), message.length());
    113110}
    114111

  • trunk/Source/WebKit/UIProcess/socket/RemoteInspectorClient.h

    r245726 r249158  
    3030#include <JavaScriptCore/RemoteControllableTarget.h>
    3131#include <JavaScriptCore/RemoteInspectorConnectionClient.h>
    32 #include <JavaScriptCore/RemoteInspectorSocketEndpoint.h>
    3332#include <wtf/HashMap.h>
    3433#include <wtf/Vector.h>
    … …  
    7978    void setBackendCommands(const Event&);
    8079
    81     void didClose(ConnectionID) override;
    82     HashMap<String, CallHandler>& dispatchMap() override;
     80    void didClose(ConnectionID) final;
     81    HashMap<String, CallHandler>& dispatchMap() final;
    8382
    8483    void sendWebInspectorEvent(const String&);
    8584
    8685    RemoteInspectorObserver& m_observer;
    87     std::unique_ptr<Inspector::RemoteInspectorSocketEndpoint> m_socket;
    8886    Optional<ConnectionID> m_connectionID;
    8987    HashMap<ConnectionID, Vector<Target>> m_targets;
Note: See TracChangeset for help on using the changeset viewer.