From 91c723e0e7fbde0b9731661a7b5cf8264519e1ac Mon Sep 17 00:00:00 2001 From: Soufiane Ghzal Date: Thu, 4 Oct 2018 18:16:35 +0200 Subject: [PATCH] fix #38 - destroyed target trying to initialize --- src/Browser.php | 16 ++-- src/Communication/Target.php | 26 +++++- .../static-web/create-and-destroy-target.html | 9 ++ test/resources/static-web/empty.js | 3 + test/suites/HttpBrowsingTest.php | 93 +++++++++++++++++++ test/suites/PageTest.php | 10 -- 6 files changed, 136 insertions(+), 21 deletions(-) create mode 100644 test/resources/static-web/create-and-destroy-target.html create mode 100644 test/resources/static-web/empty.js create mode 100644 test/suites/HttpBrowsingTest.php diff --git a/src/Browser.php b/src/Browser.php index ead3b0e8..4f7b05a4 100644 --- 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 index b4b35bba..2746aa45 100644 --- a/src/Communication/Target.php +++ b/src/Communication/Target.php @@ -19,6 +19,14 @@ class Target */ protected $session; + /** + * @var Connection + */ + private $connection; + + /** + * @var bool + */ protected $destroyed = false; /** @@ -26,10 +34,10 @@ class Target * @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 new file mode 100644 index 00000000..40afef75 --- /dev/null +++ b/test/resources/static-web/create-and-destroy-target.html @@ -0,0 +1,9 @@ + + +

Hello world

+ + + + diff --git a/test/resources/static-web/empty.js b/test/resources/static-web/empty.js new file mode 100644 index 00000000..c328db74 --- /dev/null +++ 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 new file mode 100644 index 00000000..ba109cda --- /dev/null +++ b/test/suites/HttpBrowsingTest.php @@ -0,0 +1,93 @@ +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 index 8e78c1d5..b6521c07 100644 --- 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();