net: Port NetworkContextConfigurationBrowserTest to desktop Android [chromium/src : main]
James Cook (Gerrit)
James Cook added 4 comments![]( "Open in Gerrit")
BrowserWindowInterface* CreateIncognitoBrowserWindow() {This is a cross-platform (desktop + Android) way to create an incognito window. Window creation is async on Android, hence the TestFuture.
// Android.I think this might be due to the Android emulator configuration used on the bots (I think networking is disabled at the OS level). I've seen similar networking test failures on Android in other suites.
// Use fetch() because service workers don't support XMLHttpRequest.and we have to use a service worker for manifest V3.
Related details
- Matt Mueller
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Matt Mueller (Gerrit)
Matt Mueller added 5 comments![]( "Open in Gerrit")
generally lgtm, one question about the incognito flag
// Needed to support incognito on desktop Android.is this comment accurate? Looking at https://source.chromium.org/chromium/chromium/src/+/main:components/feed/README.md I'm confused why incognito would require a "feed" feature flag.
BrowserWindowInterface* CreateIncognitoBrowserWindow() {This is a cross-platform (desktop + Android) way to create an incognito window. Window creation is async on Android, hence the TestFuture.
Acknowledged
// Android.I think this might be due to the Android emulator configuration used on the bots (I think networking is disabled at the OS level). I've seen similar networking test failures on Android in other suites.
that's weird since I would think the resolution should get caught by the TestHostResolver which shouldn't touch the real networking stack at all, but I guess there could be some layer that is saying "oh a resolution failed, and it looks like there's no networking so I'll return INTERNET_DISCONNECTED instead of the actual error"? dunno. :shrug:
// Use fetch() because service workers don't support XMLHttpRequest.and we have to use a service worker for manifest V3.
- James Cook
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
James Cook (Gerrit)
James Cook added 2 comments![]( "Open in Gerrit")
Matt, please take another look.
// Needed to support incognito on desktop Android.is this comment accurate? Looking at https://source.chromium.org/chromium/chromium/src/+/main:components/feed/README.md I'm confused why incognito would require a "feed" feature flag.
Sadly, it's required, at least for now.
See ExtensionBrowserTest:
https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/extensions/extension_browsertest.cc;drc=734942efce8a4fba157de5cb8f0ae7eec787d36b;l=191
Or BrowserWindowAndroidBrowserTestBase:
https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/browser_window/test/android/browser_window_android_browsertest_base.cc;drc=1b064913f23efc41aefe42c7a3810da86f857d91;l=16
Mobile Android can open a "window" with a mix of incognito and regular profiles. But for desktop Android we want incognito in a separate window, like Win/Mac/Linux. That's what this flag does. I don't know why it's in namespace `feed` though.
Related details
- Matt Mueller
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Matt Mueller (Gerrit)
Matt Mueller voted Code-Review+1![]( "Open in Gerrit")
- James Cook
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
James Cook (Gerrit)
James Cook voted and added 1 comment![]( "Open in Gerrit")
Votes added by James Cook
| Commit-Queue | +2 |
1 comment
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
net: Port NetworkContextConfigurationBrowserTest to desktop Android
This is part of the effort to support extensions on desktop Android.
This test suite contains a test of third-party cookie behavior for
extensions.
- Use ENABLE_EXTENSIONS_CORE instead of ENABLE_EXTENSIONS, so that
desktop Android will be included
- Eliminate usage of Browser, especially browser()->profile()
- Switch to cross-platform GetProfile() and rename an existing
GetProfile() helper method
- Use TabListInterface instead of TabStripModel
- Convert the test extension from manifest V2 to V3
- Replace XMLHttpRequest with fetch() as the former is not supported
in service workers, which MV3 requires
Flakiness checks are skipped because this test suite contains tests
with PRE_ steps and the Android flakiness detector does not support
them (and reports false positive flakes). I ran the flake detector
on an earlier patchset (see comments about flake in the CL).
- M chrome/browser/net/network_context_configuration_browsertest.cc
- M chrome/test/BUILD.gn
Code-Review: +1 by Matt Mueller
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
luci-bisection@appspot.gserviceaccount.com (Gerrit)
luci-bi...@appspot.gserviceaccount.com has created a revert of this change![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |