Switch DevTools response body collection to Durable Messages [devtools/devtools-frontend : main]
Alex N. Jose (Gerrit)
Alex N. Jose added 1 comment![]( "Open in Gerrit")
Frontend changes required for switching DevTools network panel to Durable Messages. I'll add the base::Feature to this CL, but could use an early review to make sure I'm in the right direction (my first DevTools frontend change here)
Related details
- Danil Somsikov
- Paul Irish
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Danil Somsikov (Gerrit)
Danil Somsikov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Danil Somsikov
| Code-Review | +1 |
1 comment
- Alex N. Jose
- Paul Irish
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
Alex Rudenko added 1 comment![]( "Open in Gerrit")
enableDurableMessages: true,it looks like this does not enable the overall Network domain anymore?
Related details
- Alex N. Jose
- Paul Irish
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex N. Jose (Gerrit)
Alex N. Jose added 1 comment![]( "Open in Gerrit")
enableDurableMessages: true,it looks like this does not enable the overall Network domain anymore?
That's what the test failures suggest, which is thrown from [here](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/inspector/inspector_network_agent.cc;l=2346;drc=da1f749634c9a401cc756f36c2e6ce233e1c9b4d).
Looking at the [current implementation](https://source.chromium.org/chromium/chromium/src/+/main:content/browser/devtools/protocol/network_handler.cc;l=1474-1476?q=content%2Fbrowser%2Fdevtools%2Fprotocol%2Fnetwork_handler.cc&ss=chromium%2Fchromium%2Fsrc) and the test failures' concentration around service workers, I'm suspecting that it might have something to do with a storage partition not being available, which would prevent a fall-through?
Related details
- Alex Rudenko
- Paul Irish
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex N. Jose (Gerrit)
Alex N. Jose added 1 comment![]( "Open in Gerrit")
enableDurableMessages: true,Alex N. Joseit looks like this does not enable the overall Network domain anymore?
That's what the test failures suggest, which is thrown from [here](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/inspector/inspector_network_agent.cc;l=2346;drc=da1f749634c9a401cc756f36c2e6ce233e1c9b4d).
Looking at the [current implementation](https://source.chromium.org/chromium/chromium/src/+/main:content/browser/devtools/protocol/network_handler.cc;l=1474-1476?q=content%2Fbrowser%2Fdevtools%2Fprotocol%2Fnetwork_handler.cc&ss=chromium%2Fchromium%2Fsrc) and the test failures' concentration around service workers, I'm suspecting that it might have something to do with a storage partition not being available, which would prevent a fall-through?
crrev.com/c/7004116 and crrev.com/c/7006880 will address this issue.
Related details
- Alex Rudenko
- Paul Irish
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Danil Somsikov (Gerrit)
Danil Somsikov voted Code-Review+1![]( "Open in Gerrit")
- Alex N. Jose
- Alex Rudenko
- Paul Irish
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex N. Jose (Gerrit)
Alex N. Jose added 1 comment![]( "Open in Gerrit")
@d...@chromium.org, @alexr...@chromium.org, @paul...@chromium.org, the storage partition bug has been fixed, and this feature is now behind a feature flag thats enabled by default. This CL should be good for review, PTAL!
Alex Rudenko (Gerrit)
Alex Rudenko added 1 comment![]( "Open in Gerrit")
async function setupMocks(do we need this if it is enabled by default?
Related details
- Alex N. Jose
- Paul Irish
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
Alex Rudenko added 1 comment![]( "Open in Gerrit")
async function setupMocks(if the mock is needed for a test, let's add a `describe('with durable messages')` and in the describe let's call `setup({enabledFeatures: [devToolsEnableDurableMessages]})`
Alex Rudenko (Gerrit)
Alex Rudenko voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Alex N. Jose
- Paul Irish
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex N. Jose (Gerrit)
Alex N. Jose added 2 comments![]( "Open in Gerrit")
async function setupMocks(Alex N. Josedo we need this if it is enabled by default?
Since it's feature-gated, I think tests better exercise via the feature flag (similar to Chromium's Virtual tests). If we were to switch the default to DISABLE_BY_DEFAULT, the burden will fall on the flag change to update or disable the tests otherwise.
if the mock is needed for a test, let's add a `describe('with durable messages')` and in the describe let's call `setup({enabledFeatures: [devToolsEnableDurableMessages]})`
- Alex Rudenko
- Danil Somsikov
- Paul Irish
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
Alex Rudenko voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alex Rudenko
| Code-Review | +1 |
1 comment
async function setupMocks(Alex N. Josedo we need this if it is enabled by default?
Since it's feature-gated, I think tests better exercise via the feature flag (similar to Chromium's Virtual tests). If we were to switch the default to DISABLE_BY_DEFAULT, the burden will fall on the flag change to update or disable the tests otherwise.
- Alex N. Jose
- Danil Somsikov
- Paul Irish
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Danil Somsikov (Gerrit)
Danil Somsikov voted Code-Review+1![]( "Open in Gerrit")
- Alex N. Jose
- Paul Irish
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex N. Jose (Gerrit)
Alex N. Jose added 1 comment![]( "Open in Gerrit")
@alexr...@chromium.org, @d...@chromium.org, rebasing knocked out your LGTMs. Please +1 if this looks good to land.
Related details
- Alex Rudenko
- Danil Somsikov
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
Alex Rudenko voted Code-Review+1![]( "Open in Gerrit")
- Alex N. Jose
- Danil Somsikov
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex N. Jose (Gerrit)
Alex N. Jose voted Auto-Submit+1![]( "Open in Gerrit")
- Danil Somsikov
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |