Restructure navigate-iframe-out2in.js test to properly await all network events and simplify it along the away. [chromium/src : main]
Danil Somsikov (Gerrit)
Attention is currently required from: Andrey Kosyakov.
Danil Somsikov would like Andrey Kosyakov to review this change.
Restructure navigate-iframe-out2in.js test to properly await all network events and simplify it along the away.
Bug: 1180274
Change-Id: I73e45dbbf9b9fc78a281da8848ab0b5610c5b86e
---
M third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-out2in.js
M third_party/blink/web_tests/platform/generic/http/tests/inspector-protocol/network/navigate-iframe-out2in-expected.txt
D third_party/blink/web_tests/platform/mac-mac10.13/http/tests/inspector-protocol/network/navigate-iframe-out2in-expected.txt
D third_party/blink/web_tests/platform/mac-mac10.14/http/tests/inspector-protocol/network/navigate-iframe-out2in-expected.txt
4 files changed, 58 insertions(+), 61 deletions(-)
To view, visit change 3726091. To unsubscribe, or for help writing mail filters, visit settings.
Danil Somsikov (Gerrit)
Attention is currently required from: Andrey Kosyakov.
Patch set 4:Auto-Submit +1Commit-Queue +1
Andrey Kosyakov (Gerrit)
Attention is currently required from: Danil Somsikov.
4 comments:
File third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-out2in.js:
Patch Set #4, Line 14:
return true;So we'll return true regardless of what check returns? I guess this isn't quite "matcher" then? If we just want a certain handler invoked on all events, why don't we just install it independently with another set of network.on<event>(handler)?
Patch Set #4, Line 36:
if (!iframeEventsExpected)I think ExtraInfo may actually come to either parent or child target :-(
Patch Set #4, Line 45:
await networkRequestEvents(dp.Network); // iframeSo we expect all frame events to arrive after all main frame events. I'm afraid we really can't count on this, because of ...ExtraInfo events that use a completely independent path and hence race against other events. So you may get requestWillBeSent from frame once the body has been fetched and parsed and yet ResponseReceivedExtraInfo has not arrived yet.
Patch Set #4, Line 49:
networkRequestEvents(As per the first comment, let's just explicitly set up handlers with do.Network.on...() here for better readability.
To view, visit change 3726091. To unsubscribe, or for help writing mail filters, visit settings.
Danil Somsikov (Gerrit)
Attention is currently required from: Andrey Kosyakov.
Patch set 6:Auto-Submit +1
4 comments:
File third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-out2in.js:
Patch Set #4, Line 14:
return true;
So we'll return true regardless of what check returns? I guess this isn't quite "matcher" then? If w […]
Done
Patch Set #4, Line 36:
if (!iframeEventsExpected)I think ExtraInfo may actually come to either parent or child target :-(
Is this the behavior we want to codify? If I understand the intention of the original CL at crrev.com/c/1948898 we wanted to use the test to fix the issue.
Patch Set #4, Line 45:
await networkRequestEvents(dp.Network); // iframe
So we expect all frame events to arrive after all main frame events. […]
Done
Patch Set #4, Line 49:
networkRequestEvents(
As per the first comment, let's just explicitly set up handlers with do.Network.on... […]
Done
To view, visit change 3726091. To unsubscribe, or for help writing mail filters, visit settings.
Danil Somsikov (Gerrit)
Attention is currently required from: Andrey Kosyakov.
1 comment:
Patchset:
Friendly ping:)
To view, visit change 3726091. To unsubscribe, or for help writing mail filters, visit settings.
Andrey Kosyakov (Gerrit)
Attention is currently required from: Danil Somsikov.
Patch set 6:Code-Review +1
3 comments:
Patchset:
lgtm with comments
File third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-out2in.js:
Patch Set #4, Line 36:
if (!iframeEventsExpected)Is this the behavior we want to codify? If I understand the intention of the original CL at crrev. […]
I'm not suggesting to codify this behavior, but I'm not sure we can get away with pretending it doesn't exist in the test (we can see if the test is stable as is, perhaps). I think the right thing here is to do what we tell clients to do in this case, i.e. since we don't know on which target the event will arrive, accept it from any target (conveniently, the request id is guid in this case, so it doesn't have to be target-specific). This is what the front-end does, too, since the requests from all targets/sessions are funneled through global network request manager.
File third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-out2in.js:
Patch Set #6, Line 74:
await new Promise((r)=>setTimeout(r,1000));Please remove!
To view, visit change 3726091. To unsubscribe, or for help writing mail filters, visit settings.
Danil Somsikov (Gerrit)
Patch set 9:Auto-Submit +1
1 comment:
File third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-out2in.js:
Patch Set #6, Line 74:
await new Promise((r)=>setTimeout(r,1000));Please remove!
Oops:)
To view, visit change 3726091. To unsubscribe, or for help writing mail filters, visit settings.
Danil Somsikov (Gerrit)
Patch set 9:Commit-Queue +2
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
6 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-out2in.js
Insertions: 0, Deletions: 1.
The diff is too large to show. Please review the diff.
```
Restructure navigate-iframe-out2in.js test to properly await all network events and simplify it along the away.
Bug: 1180274
Change-Id: I73e45dbbf9b9fc78a281da8848ab0b5610c5b86e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3726091
Auto-Submit: Danil Somsikov <d...@chromium.org>
Commit-Queue: Danil Somsikov <d...@chromium.org>
Reviewed-by: Andrey Kosyakov <ca...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1019662}
---
M third_party/blink/web_tests/http/tests/inspector-protocol/network/navigate-iframe-out2in.js
M third_party/blink/web_tests/platform/generic/http/tests/inspector-protocol/network/navigate-iframe-out2in-expected.txt
D third_party/blink/web_tests/platform/mac-mac10.13/http/tests/inspector-protocol/network/navigate-iframe-out2in-expected.txt
D third_party/blink/web_tests/platform/mac-mac10.14/http/tests/inspector-protocol/network/navigate-iframe-out2in-expected.txt
4 files changed, 79 insertions(+), 63 deletions(-)