Open Bug 1830875 Opened 1 year ago Updated 14 days ago

[meta] Support WebDriver BiDi in Browsertime

Categories

(Remote Protocol :: WebDriver BiDi, task)

Product:
Remote Protocol
Component:
WebDriver BiDi
Type:
task

Tracking

(Not tracked)

People

(Reporter: whimboo, Unassigned, NeedInfo)

References

(Depends on 2 open bugs, Blocks 1 open bug,
URL
)

Details

(Keywords: meta)

Upstream issue: https://github.com/sitespeedio/browsertime/issues/1170#issuecomment-1521346069

We have to support Browsertime so that it can switch to using the WebDriver BiDi implementation.

Given that it is a node package it actually relies on selenium-webdriver. So we should make sure to get all the required features added there.

Peter, is the list of features from the related GitHub issue still correct or has it been changed since the issue got filed? Maybe you could list everything that is needed? Thanks!

Flags: needinfo?(peter)

Hi Henrik, the list is correct. To get rid off the extension we use we need to add:

  • Set request headers
  • Set cookies
  • Inject JS on the page on new document
  • Set basic auth (same as request headers)
  • Block domains
  • Clear browser cache.

For the ones that already been added, is there any documentation I can use so I can implement in Browsertime?

Flags: needinfo?(peter)

Thanks Peter! I've already added dependencies for already filed meta bugs. I'll add more once we have the appropriate spec issues up and related meta bugs filed as well.

Depends on: 1826191, 1826196
Depends on: 1850680, 1854574
Depends on: 1854582
No longer depends on: 1854574
Depends on: 1744483
See Also: → 1613883

(In reply to Peter Hedenskog from comment #1)

Hi Henrik, the list is correct. To get rid off the extension we use we need to add:

  • Set request headers
  • Set cookies
  • Inject JS on the page on new document
  • Set basic auth (same as request headers)
  • Block domains
  • Clear browser cache.

Hi Peter. Beside Clear browser cache everything should be available now with the caveat that some commands do not support all the arguments, eg. network interception only allows to intercept and continue/fail without modifying any headers and body. If that is needed please tell us.

Was anything else added to browsertime which would require even additional APIs?

Flags: needinfo?(peter)

Woho, great news :) So I can start implement them, is there any documentation of the actual commands anywhere? Or just a list with the commands, I can try to figure out the rest.

Was anything else added to browsertime which would require even additional APIs?

Let me get back tomorrow, I need to go through the code to check so I haven't missed anything.

An overview of the current implementation status for Firefox you can get via this Google Spreadsheet - there is another sheet for the events as well. Calling conventions and payload information can then all be found in the referenced section of the WebDriver BiDi specification.

Let me or Julian know if you need more information.

Thanks for the document! Maybe there are tests that I can look at to see some implementation?

There are some that I'm unsure how to map in the document:

  • Set request headers - in the CDP that is Network.setExtraHTTPHeaders and in the extension we use for Firefox https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/onBeforeSendHeaders - there's no oneliner here but this should work if we use network.addIntercept and then network.continueRequest and there add the extra header?

  • Inject JS on the page on new document - in CDP that is Page.addScriptToEvaluateOnNewDocument and in the Firefox extension today that is window.browser.contentScripts.register. Is the equivalent script.addPreloadScript? But that add script on a new window and not new document or is it the same?

  • Block domains - in the CDP world there's no perfect match, there we use Network.setBlockedURLs to block URLs and then on domain level, we use a command line flag --host-resolver-rules to set per domain. In the extension for Firefox we use window.browser.webRequest.onBeforeSendHeaders. I can not find a matching thing in the document? Maybe there's something like host-resolver-rules we can use for Firefox too? The thing is that we want to do this without overhead, since the main use case is to have a simple way to block 3rd party content and measure the performance without that.

(In reply to Peter Hedenskog from comment #6)

Thanks for the document! Maybe there are tests that I can look at to see some implementation?

Sure. For all the BiDi modules, which are implemented, you can find tests under the following location:
https://github.com/web-platform-tests/wpt/tree/master/webdriver/tests/bidi

The utilized Python clients for WebDriver classic and BiDi are at:
https://github.com/web-platform-tests/wpt/tree/master/tools/webdriver

Let me know if you need more infos.

This is a feature that actually doesn't work yet. While we have basic support for network.continueRequest implemented you cannot yet set headers. This requires bug 1850680 (already on the dependency list) to be fixed first.

  • Inject JS on the page on new document - in CDP that is Page.addScriptToEvaluateOnNewDocument and in the Firefox extension today that is window.browser.contentScripts.register. Is the equivalent script.addPreloadScript? But that add script on a new window and not new document or is it the same?

Yes, that is the equivalent command to CDP. It registers scripts to be run on document creation. Note that a script that gets registered will not immediately run on the current document but requires at least a reload of the page.

  • Block domains - in the CDP world there's no perfect match, there we use Network.setBlockedURLs to block URLs and then on domain level, we use a command line flag --host-resolver-rules to set per domain. In the extension for Firefox we use window.browser.webRequest.onBeforeSendHeaders. I can not find a matching thing in the document? Maybe there's something like host-resolver-rules we can use for Firefox too? The thing is that we want to do this without overhead, since the main use case is to have a simple way to block 3rd party content and measure the performance without that.

Isn't that a feature that can also be implemented via network interception? With adding an interception you can pass in a list of URLPatterns that this intercept should handle. Then when a network.beforeRequestSent event is getting send you can check if your intercept is contained in the intercepts list and execute network.failRequest. This should already work.

Redirect a needinfo that is pending on an inactive user to the triage owner.
:whimboo, since the bug has recent activity, could you please find another way to get the information or close the bug as INCOMPLETE if it is not actionable?

For more information, please visit BugBot documentation.

Flags: needinfo?(peter) → needinfo?(hskupin)

Not sure why the account from Peter is inactive. I've contacted him via Matrix. Lets see how to follow-up further.

Flags: needinfo?(hskupin)

Sorry I use both, been busy with a lot of other stuff and didn't answer.

Let me start then with the inject JS and take them one by one.

(In reply to Peter Hedenskog from comment #10)

Sorry I use both, been busy with a lot of other stuff and didn't answer.

No worries. I thought so! :) I was only worried about the inactive state of the account, which should not be the case in such a short time frame.

Let me start then with the inject JS and take them one by one.

Sounds good. That is a feature that we basically fully support. As usual let us know if there are issues or questions.

With the new browsertime release 21.6.0 it's using now both cookies and HTTPAuth via BiDi.

With the new browsertime release 21.7.0 WebDriver BiDi is used now for blocking domains.

From the Browsertime 22.0.0 release notes:

Removed the Browsertime extension. In the old days the extension helped Chrome and Firefox to add cookies, requestheaders, clear caches and more. However all that functionality has been implemented in CDP for Chrome and most functionality using Bidi webdriver for Firefox. At the moment we drop two things for Firefox: Add request header and clear the cache inside of scripting. We hope both soon is supported in Bidi #2124.

For request headers we are close for landing. So it's just the clear cache feature we do not have an equivalent for now.

Hi Peter! Setting request headers should work now in the recent Nightly build for network.continueRequest. If you could try it out how it works for you we would appreciate your feedback. Thanks!

Wow great, thank you Henrik! Can you point me to a test case or if there are any documentation and I'll give it a try?

Sure, you can find several examples over here:
https://github.com/web-platform-tests/wpt/tree/master/webdriver/tests/bidi/network/continue_request

Let us know if something is unclear. Documentation on MDN we do not have yet.

Thank you Henrik! I did try it out but didn't get it to work, I think I'm doing something wrong. I tested with FF Nightly 129.0a1 (2024-06-19).

The code is here:
https://github.com/sitespeedio/browsertime/pull/2108/files#diff-8b557fbe5e08f164beffe28de4197ec644d0b6c44458a3704502c0a6d7025213

What happens is that network.addIntercept and network.continueRequest works fine (I get type success) but the navigation never moves on, it seems I'm missing something in the continue request?

(In reply to Peter Hedenskog from comment #18)

The code is here:
https://github.com/sitespeedio/browsertime/pull/2108/files#diff-8b557fbe5e08f164beffe28de4197ec644d0b6c44458a3704502c0a6d7025213

What happens is that network.addIntercept and network.continueRequest works fine (I get type success) but the navigation never moves on, it seems I'm missing something in the continue request?

Is there a way to easily run this code? Maybe a small example test / script that we could utilize?

Flags: needinfo?(peter)

I had a look at the code, and it seems fine from a highlevel perspective. If you share instructions on how to test it I should be able to debug what's wrong.

Edit: Ok, I think I can repro now, just have to pass --requestheader

Will check why this fails

Flags: needinfo?(jdescottes)

I get a really weird behavior when testing this, the channel seems to be resumed properly but the test stops immediately with

[2024-06-20 18:20:05] ERROR: Failed waiting on page https://dashboard.sitespeed.io/d/9NDMzFfMk/page-metrics-desktop?orgId=1&var-base=sitespeed_io&var-path=desktop&var-testname=spa&var-group=dashboard_sitespeed_io&var-page=pageTimingMetricsDefault&var-browser=chrome&var-connectivity=cable&var-function=median&var-resulturl=https:%2F%2Fdata.sitespeed.io%2F&var-screenshottype=jpg 
to finished loading, timed out after 120000 ms WebDriverError: Failed to decode response from marionette

The 120000ms is odd because the test runs in under 5 seconds on my machine. Peter do you see the same thing?

Flags: needinfo?(jdescottes)

Ok I think I know what is wrong. By definition BiDi continueRequest will override ALL headers when "headers" is provided, it doesn't merge them with the current request headers.

So if you pass headers: [...parsedEvent.params.request.headers, ...headers], instead of just headers I think it should work.

I'm still trying to identify clearly why it makes the test fail with such an unhelpful error message though.

I was running the test with a debug build, so I think in my case the test was stopping early because I was hitting a very early assert at https://searchfox.org/mozilla-central/rev/74518d4f6979b088e28405ba7e6238f4707639cf/netwerk/protocol/http/Http2Stream.cpp#65

It's likely that with a regular build, it simply hangs / doesn't properly resume the request.
I can easily reproduce this when testing manually with a bidi client, but I can't reproduce it with a regular wdspec test for now.

Edit: This specific issue is most likely related to http2 here, which we don't use by default in wdspec tests

Thank you Julian, merging the headers worked for me!

Thanks for checking that Julian. A hang resulting from not using all the headers is not great. Especially when there is no additional information why it happens. Even the early assert doesn't really provide a message. We probably want to further investigate and discuss this scenario. Maybe we should handle that on a new bug?

Flags: needinfo?(peter) → needinfo?(jdescottes)
Flags: needinfo?(jdescottes)
See Also: → 1904317

Hi Peter, one question to the initially asked feature for clear browser cache. Maybe you can give some more details?

We now have a preliminary implementation for network.setCacheBypass (the related spec is not merged yet - so small changes could still happen) which basically allows you to disable/enable the usage of the Network cache per top-level browsing context. Would that meet your expectations? If yes we should hopefully have a final solution soon.

Flags: needinfo?(peter)

Cool!

The use case is clearing the cache from Browsertime scripting. I've been mainly using it for logged in scenarios where you log in the user (have the login cookie) but you clear the browser file cache. That way you could simulate a user that is logged in (the login cookie is valid) but assets (JS/CSS/Images etc) has expired within one scripting file.

Flags: needinfo?(peter)

Peter, from your initial list of requested features (see comment 1), is there anything that still isn’t fully functional? Are there any other features you need that require using an alternative method, such as a web extension, to complete the task?

Flags: needinfo?(peter)
You need to log in before you can comment on or make changes to this bug.