diff --git a/src/ChromeDevtoolsProtocol/DevtoolsClient.php b/src/ChromeDevtoolsProtocol/DevtoolsClient.php index 4cfae26f..7441b4d8 100644 --- a/src/ChromeDevtoolsProtocol/DevtoolsClient.php +++ b/src/ChromeDevtoolsProtocol/DevtoolsClient.php @@ -34,12 +34,18 @@ class DevtoolsClient implements DevtoolsClientInterface, InternalClientInterface /** @var object[][] */ private $eventBuffers = []; - public function __construct(string $wsUrl) + public function __construct(WebSocketClient $wsClient) { - $this->wsClient = new WebSocketClient($wsUrl, "http://" . parse_url($wsUrl, PHP_URL_HOST)); - if (!$this->wsClient->connect()) { + $this->wsClient = $wsClient; + } + + public static function createFromDebuggerUrl(string $wsUrl) + { + $wsClient = new WebSocketClient($wsUrl, "http://" . parse_url($wsUrl, PHP_URL_HOST)); + if (!$wsClient->connect()) { throw new RuntimeException(sprintf("Could not connect to [%s].", $wsUrl)); } + return new self($wsClient); } public function __destruct() @@ -103,27 +109,21 @@ public function awaitEvent(ContextInterface $ctx, string $method) $this->eventBuffers = []; for (; ;) { - $eventMessage = null; - $this->getWsClient()->setDeadline($ctx->getDeadline()); foreach ($this->getWsClient()->receive() ?: [] as $payload) { /** @var Payload $payload */ $message = json_decode($payload->getPayload()); - $nextEventMessage = $this->handleMessage($message, $eventMessage === null ? $method : null); - - if ($nextEventMessage !== null) { - $eventMessage = $nextEventMessage; - } + $this->handleMessage($message); } - if ($eventMessage !== null) { - return $eventMessage->params; + if (!empty($this->eventBuffers[$method])) { + return array_shift($this->eventBuffers[$method])->params; } } } - private function handleMessage($message, ?string $returnIfEventMethod = null) + private function handleMessage($message) { if (isset($message->error)) { throw new ErrorException($message->error->message, $message->error->code); @@ -135,14 +135,10 @@ private function handleMessage($message, ?string $returnIfEventMethod = null) } } - if ($returnIfEventMethod !== null && $message->method === $returnIfEventMethod) { - return $message; - } else { - if (!isset($this->eventBuffers[$message->method])) { - $this->eventBuffers[$message->method] = []; - } - array_push($this->eventBuffers[$message->method], $message); + if (!isset($this->eventBuffers[$message->method])) { + $this->eventBuffers[$message->method] = []; } + array_push($this->eventBuffers[$message->method], $message); } else if (isset($message->id)) { $this->commandResults[$message->id] = $message->result ?? new \stdClass(); diff --git a/src/ChromeDevtoolsProtocol/Instance/Instance.php b/src/ChromeDevtoolsProtocol/Instance/Instance.php index 3954bc44..e66c0a76 100644 --- a/src/ChromeDevtoolsProtocol/Instance/Instance.php +++ b/src/ChromeDevtoolsProtocol/Instance/Instance.php @@ -103,7 +103,7 @@ public function createSession(ContextInterface $ctx, string $url = "about:blank" throw new RuntimeException("No browser debugger URL. Try upgrading to newer version of Chrome."); } - $browser = new DevtoolsClient($version->webSocketDebuggerUrl); + $browser = DevtoolsClient::createFromDebuggerUrl($version->webSocketDebuggerUrl); try { $browserContextId = $browser->target()->createBrowserContext($ctx)->browserContextId; try { diff --git a/src/ChromeDevtoolsProtocol/Instance/Tab.php b/src/ChromeDevtoolsProtocol/Instance/Tab.php index a0c7ea2c..25d67784 100644 --- a/src/ChromeDevtoolsProtocol/Instance/Tab.php +++ b/src/ChromeDevtoolsProtocol/Instance/Tab.php @@ -60,7 +60,7 @@ public function __construct($data, InternalInstanceInterface $internalInstance) */ public function devtools(): DevtoolsClientInterface { - return new DevtoolsClient($this->webSocketDebuggerUrl); + return DevtoolsClient::createFromDebuggerUrl($this->webSocketDebuggerUrl); } /** diff --git a/src/ChromeDevtoolsProtocol/Session.php b/src/ChromeDevtoolsProtocol/Session.php index 0e385569..cbc2cee4 100644 --- a/src/ChromeDevtoolsProtocol/Session.php +++ b/src/ChromeDevtoolsProtocol/Session.php @@ -114,21 +114,15 @@ public function awaitEvent(ContextInterface $ctx, string $method) } for (; ;) { - $eventMessage = null; - $receivedMessage = $this->browser->target()->awaitReceivedMessageFromTarget($ctx); if ($receivedMessage->targetId === $this->targetId && $receivedMessage->sessionId === $this->sessionId) { - $nextEventMessage = $this->handleMessage(json_decode($receivedMessage->message), $method); - - if ($eventMessage === null && $nextEventMessage !== null) { - $eventMessage = $nextEventMessage; - } + $this->handleMessage(json_decode($receivedMessage->message)); } - if ($eventMessage !== null) { - return $eventMessage->params; + if (!empty($this->eventBuffers[$method])) { + return array_shift($this->eventBuffers[$method])->params; } } } @@ -136,7 +130,7 @@ public function awaitEvent(ContextInterface $ctx, string $method) /** * @internal */ - private function handleMessage($message, ?string $returnIfEventMethod = null) + private function handleMessage($message) { if (isset($message->error)) { throw new ErrorException($message->error->message, $message->error->code); @@ -148,14 +142,10 @@ private function handleMessage($message, ?string $returnIfEventMethod = null) } } - if ($returnIfEventMethod !== null && $message->method === $returnIfEventMethod) { - return $message; - } else { - if (!isset($this->eventBuffers[$message->method])) { - $this->eventBuffers[$message->method] = []; - } - array_push($this->eventBuffers[$message->method], $message); + if (!isset($this->eventBuffers[$message->method])) { + $this->eventBuffers[$message->method] = []; } + array_push($this->eventBuffers[$message->method], $message); } else if (isset($message->id)) { $this->commandResults[$message->id] = $message->result ?? new \stdClass(); diff --git a/test/ChromeDevtoolsProtocol/DevtoolsClientTest.php b/test/ChromeDevtoolsProtocol/DevtoolsClientTest.php index d4f9371c..efbcc07e 100644 --- a/test/ChromeDevtoolsProtocol/DevtoolsClientTest.php +++ b/test/ChromeDevtoolsProtocol/DevtoolsClientTest.php @@ -1,15 +1,20 @@ getMockBuilder(Payload::class) + ->disableOriginalConstructor() + ->getMock(); + $payloadStub->method('getPayload')->willReturn($payload); + $responses[] = $payloadStub; + }; + + // create a stub websocket client, which returns predefined messages + $wsClient = $this->getMockBuilder(WebSocketClient::class) + ->disableOriginalConstructor() + ->getMock(); + $wsClient->method('receive')->willReturnCallback(function() use(&$responses) { + if (!empty($responses)) { + return [array_shift($responses)]; + } + return null; + }); + $wsClient->method('setDeadline')->willReturnCallback(function($deadline) { + $timeout = floatval($deadline->format("U.u")) - microtime(true); + if ($timeout < 0.0) { + throw new DeadlineException("Socket deadline reached."); + } + }); + + // create the failing scenario + $ctx = Context::withTimeout(Context::background(), 3); + + $client = new DevtoolsClient($wsClient); + register_shutdown_function(function () use ($client) { $client->close(); }); + + $addResponse('{"id":1,"result":{}}'); + $client->page()->enable($ctx); + + $addResponse('{"id":2,"result":{}}'); + $client->network()->enable($ctx, EnableRequest::make()); + + $client->network()->addLoadingFinishedListener(function (LoadingFinishedEvent $event) use($ctx, $client, $addResponse) { // <- 2 + + // The order of these responses matters, if the loadEventFired comes first, awaitLoadEventFired (1) would wait forever + $addResponse('{"method":"Page.loadEventFired","params":{"timestamp":6758.846787}}'); + $addResponse('{"id":4,"result":{"body":"..."}}'); + + $client->network()->getResponseBody($ctx, GetResponseBodyRequest::builder() // <- 3 + ->setRequestId($event->requestId) + ->build() + ); + }); + + $url = 'https://www.google.com'; + + $addResponse('{"id":3,"result":{"frameId":"1E56ACDD9B3B7F678F972C0EF0782649","loaderId":"AAF889CAE5B10663CA8D383A6125AC1B"}}'); + $client->page()->navigate($ctx, NavigateRequest::builder()->setUrl($url)->build()); + + $addResponse('{"method":"Network.loadingFinished","params":{"requestId":"AAF889CAE5B10663CA8D383A6125AC1B","timestamp":6758.623335,"encodedDataLength":67174,"shouldReportCorbBlocking":false}}'); + $client->page()->awaitLoadEventFired($ctx); // <- 1 + + $this->assertTrue((bool)'No conflict'); + + /* + + 1) we are waiting for the LoadEvent, but in the meanwhile the LoadingFinishedEvent arrives -> + 2) so the listener is called + 3) getResponseBody command is executed + - if the response is received before LoadEvent, everything is fine + - but if the LoadEvent is happening before the response is received, + the LoadEvent is dropped (not buffered) and awaitLoadEventFired could never return + + */ + } + + }