Bug 162099

Summary:

Web Inspector: adopt Object.awaitEvent in LayoutTests/inspector/network

Product:

WebKit

Reporter:

Devin Rousso <hi>

Component:

Web Inspector

Assignee:

Devin Rousso <hi>

Status:

REOPENED ---

Severity:

Normal

CC:

ap, commit-queue, inspector-bugzilla-changes, ryanhaddad, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

161634

Attachments:

Description	Flags
Patch	none

Description Devin Rousso 2016-09-16 16:41:05 PDT

Comment 1 Radar WebKit Bug Importer 2016-09-16 16:41:53 PDT

<rdar://problem/28346596>

Comment 2 Devin Rousso 2016-09-16 16:45:17 PDT

Created attachment 289137 [details]
Patch

Comment 3 BJ Burg 2016-09-19 07:56:18 PDT

Comment on attachment 289137 [details]
Patch

r=me

Comment 4 WebKit Commit Bot 2016-09-19 13:47:29 PDT

Comment on attachment 289137 [details]
Patch

Clearing flags on attachment: 289137

Committed r206112: <http://trac.webkit.org/changeset/206112>

Comment 5 WebKit Commit Bot 2016-09-19 13:47:33 PDT

All reviewed patches have been landed.  Closing bug.

Comment 6 Ryan Haddad 2016-09-22 11:22:06 PDT

Reverted r206112 for reason:

This change made inspector/network/xhr-json-blob-has-content.html very flaky.

Committed r206263: <http://trac.webkit.org/changeset/206263>

Comment 7 Ryan Haddad 2016-09-22 11:23:02 PDT

*** Bug 162360 has been marked as a duplicate of this bug. ***

Comment 8 Joseph Pecoraro 2016-09-22 12:07:02 PDT

Comment on attachment 289137 [details]
Patch

I really find these changes much harder to read then the original test. The original test read:

  "do this, cause and do this"

Now they read:

  "prepare for some stuff, do this"

Am I alone in this?

Comment 9 BJ Burg 2016-09-22 14:06:01 PDT

(In reply to comment #8)
> Comment on attachment 289137 [details]
> Patch
> 
> I really find these changes much harder to read then the original test. The
> original test read:
> 
>   "do this, cause and do this"
> 
> Now they read:
> 
>   "prepare for some stuff, do this"
> 
> Am I alone in this?

I think so? It's not always possible to make the test linear like that, then the promise chain kicks off at some random awkward part of the test. I prefer making the test explicitly event-based with the trigger at the end rather than implicitly depending on evaluating script being asynchronous. That way, the test is structured the same way (prep + trigger) whether the initial part of the promise chain is triggered by synchronous events from WI.Object or from async backend commands that have responses or cause events.