fix #38 - destroyed target trying to initialize

chrome-php · Oct 4, 2018 · 91c723e · 91c723e
1 parent 7c51a03
commit 91c723e
Show file tree

Hide file tree

Showing 6 changed files with 136 additions and 21 deletions.
diff --git a/src/Browser.php b/src/Browser.php
@@ -38,14 +38,8 @@ public function __construct(Connection $connection)
         // listen for target created
         $this->connection->on(Connection::EVENT_TARGET_CREATED, function (array $params) {
 
-            // create a session for the target
-            $session = $this->connection->createSession($params['targetInfo']['targetId']);
-
             // create and store the target
-            $this->targets[$params['targetInfo']['targetId']] = new Target(
-                $params['targetInfo'],
-                $session
-            );
+            $this->targets[$params['targetInfo']['targetId']] = new Target($params['targetInfo'], $this->connection);
         });
 
         // listen for target info changed
@@ -168,4 +162,12 @@ public function getTarget($targetId)
         }
         return $this->targets[$targetId];
     }
+
+    /**
+     * @return Target[]
+     */
+    public function getTargets()
+    {
+        return array_values($this->targets);
+    }
 }
diff --git a/src/Communication/Target.php b/src/Communication/Target.php
@@ -19,17 +19,25 @@ class Target
      */
     protected $session;
 
+    /**
+     * @var Connection
+     */
+    private $connection;
+
+    /**
+     * @var bool
+     */
     protected $destroyed = false;
 
     /**
      * Target constructor.
      * @param array $targetInfo
      * @param Session $session
      */
-    public function __construct(array $targetInfo, Session $session)
+    public function __construct(array $targetInfo, Connection $connection)
     {
         $this->targetInfo = $targetInfo;
-        $this->session = $session;
+        $this->connection = $connection;
     }
 
     /**
@@ -40,6 +48,12 @@ public function getSession(): Session
         if ($this->destroyed) {
             throw new TargetDestroyed('The target was destroyed.');
         }
+
+        // if not already done, create a session for the target
+        if (!$this->session) {
+            $this->session = $session = $this->connection->createSession($this->getTargetInfo('targetId'));
+        }
+
         return $this->session;
     }
 
@@ -52,8 +66,12 @@ public function destroy()
         if ($this->destroyed) {
             throw new TargetDestroyed('The target was already destroyed.');
         }
-        $this->session->destroy();
-        $this->session = null;
+
+        if ($this->session) {
+            $this->session->destroy();
+            $this->session = null;
+        }
+
         $this->destroyed = true;
     }
 

diff --git a/test/resources/static-web/create-and-destroy-target.html b/test/resources/static-web/create-and-destroy-target.html
@@ -0,0 +1,9 @@
+<html>
+    <body>
+        <h1>Hello world</h1>
+        <script>
+            navigator.serviceWorker.register('./empty.js');
+        </script>
+
+    </body>
+</html>
diff --git a/test/resources/static-web/empty.js b/test/resources/static-web/empty.js
@@ -0,0 +1,3 @@
+(function() {
+    var a = 'a';
+})();
diff --git a/test/suites/HttpBrowsingTest.php b/test/suites/HttpBrowsingTest.php
@@ -0,0 +1,93 @@
+<?php
+/**
+ * @license see LICENSE
+ */
+
+namespace HeadlessChromium\Test;
+
+use HeadlessChromium\Browser;
+use HeadlessChromium\BrowserFactory;
+use HeadlessChromium\Communication\Connection;
+
+/**
+ * @covers \HeadlessChromium\Browser
+ * @covers \HeadlessChromium\PageUtils\CookiesGetter
+ * @covers \HeadlessChromium\Page
+ */
+class HttpBrowsingTest extends HttpEnabledTestCase
+{
+
+    /**
+     * @var Browser\ProcessAwareBrowser
+     */
+    public static $browser;
+
+    public function setUp()
+    {
+        parent::setUp();
+        $factory = new BrowserFactory();
+        self::$browser = $factory->createBrowser();
+    }
+
+    public function tearDown()
+    {
+        parent::tearDown();
+        self::$browser->close();
+    }
+
+    private function openSitePage($file)
+    {
+        $page = self::$browser->createPage();
+        $page->navigate($this->sitePath($file))->waitForNavigation();
+
+        return $page;
+    }
+
+
+    /**
+     * @link https://github.com/chrome-php/headless-chromium-php/issues/38
+     */
+    public function testServiceWorkerInstantlyDestroying()
+    {
+        $factory = new BrowserFactory();
+        $browser = $factory->createBrowser();
+
+        $page = $browser->createPage();
+        $page->navigate($this->sitePath('create-and-destroy-target.html'))->waitForNavigation();
+
+        $page = $browser->createPage();
+        $page->navigate($this->sitePath('create-and-destroy-target.html'))->waitForNavigation();
+
+        // helper to track that service worker was created
+        $helper = new \stdClass();
+        $helper->created = false;
+        $helper->targetId = null;
+        $helper->destroyed = false;
+
+        // track created
+        $page->getSession()->getConnection()->on(Connection::EVENT_TARGET_CREATED, function ($e) use ($helper) {
+            if (isset($e['targetInfo']['type']) && $e['targetInfo']['type'] == 'service_worker') {
+                $helper->created = true;
+                $helper->targetId = $e['targetInfo']['targetId'];
+            }
+        });
+
+        // track destroyed
+        $page->getSession()->getConnection()->on(Connection::EVENT_TARGET_DESTROYED, function ($e) use ($helper) {
+            if ($e['targetId'] == $helper->targetId) {
+                $helper->destroyed = true;
+            }
+        });
+
+        sleep(1);
+
+        // do something to trigger service worker create/destroy
+        $page->evaluate('document.language')->getReturnValue();
+
+        // assert created/destroyed
+        $this->assertTrue($helper->created);
+        $this->assertTrue($helper->destroyed);
+        // Note: if that stops to assert, it could mean that chrome changed the way it works and that this test is
+        // no longer required
+    }
+}
diff --git a/test/suites/PageTest.php b/test/suites/PageTest.php
@@ -18,16 +18,6 @@
 class PageTest extends BaseTestCase
 {
 
-    public function testPage()
-    {
-        $connection = new Connection(new MockSocket());
-        $session = new Session('foo', 'bar', $connection);
-        $target = new Target([], $session);
-        $page = new Page($target, []);
-
-        $this->assertSame($session, $page->getSession());
-    }
-
     public function testSetViewport()
     {
         $factory = new BrowserFactory();