Fix for #14 - Buffer events always and check buffer always after socket polling

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();

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 {

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);
 	}
 
 	/**

src/ChromeDevtoolsProtocol/Session.php

@@ -114,29 +114,23 @@ 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;
 			}
 		}
 	}
 
 	/**
 	 * @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();

test/ChromeDevtoolsProtocol/DevtoolsClientTest.php

@@ -1,15 +1,20 @@
 <?php
 namespace ChromeDevtoolsProtocol;
 
+use ChromeDevtoolsProtocol\Exception\DeadlineException;
 use ChromeDevtoolsProtocol\Exception\ErrorException;
 use ChromeDevtoolsProtocol\Instance\Launcher;
 use ChromeDevtoolsProtocol\Model\Network\EnableRequest;
+use ChromeDevtoolsProtocol\Model\Network\GetResponseBodyRequest;
+use ChromeDevtoolsProtocol\Model\Network\LoadingFinishedEvent;
 use ChromeDevtoolsProtocol\Model\Page\NavigateRequest;
 use ChromeDevtoolsProtocol\Model\Security\CertificateErrorActionEnum;
 use ChromeDevtoolsProtocol\Model\Security\CertificateErrorEvent;
 use ChromeDevtoolsProtocol\Model\Security\HandleCertificateErrorRequest;
 use ChromeDevtoolsProtocol\Model\Security\SetOverrideCertificateErrorsRequest;
+use ChromeDevtoolsProtocol\WebSocket\WebSocketClient;
 use PHPUnit\Framework\TestCase;
+use Wrench\Payload\Payload;
 
 class DevtoolsClientTest extends TestCase
 {
@@ -80,4 +85,83 @@ public function testErrorHandling()
 		}
 	}
 
+	/*
+	 * https://github.com/jakubkulhan/chrome-devtools-protocol/issues/14
+	 */
+	public function testConflictBetweenAwaitAndExecute()
+	{
+		// websocket messages
+		$responses = [];
+		$addResponse = function($payload) use(&$responses) {
+			$payloadStub = $this->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
+
+		*/
+	}
+
+
 }