Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support HttpEngine openConnection(URL) #8973

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yschimke
Copy link

@yschimke yschimke commented Apr 6, 2024

Overview

First attempt at #8956.

For discussion and advice on structure and tests.

Proposed Changes

Bridge to the JavaCronetProvider from cronet-fallback, that just uses URLConnection.

@yschimke yschimke changed the title Support HttpEngine construction Support HttpEngine openConnection(URL) Apr 6, 2024
@yschimke yschimke marked this pull request as ready for review April 6, 2024 12:46
@yschimke yschimke force-pushed the httpengine branch 2 times, most recently from 179cd7b to 766cf0a Compare April 6, 2024 12:54
@utzcoz
Copy link
Member

utzcoz commented Apr 6, 2024

@yschimke There are some CI checking, including code style, commit validation. You can find them by checking GitHub Actions configurations.

@yschimke yschimke force-pushed the httpengine branch 5 times, most recently from 943976c to 9c67d17 Compare April 6, 2024 15:25
@utzcoz
Copy link
Member

utzcoz commented Apr 7, 2024

@yschimke @hoisie What about adding CI jobs to run ctesque local tests with multiple platforms, including macOS, Windows and Linux?

@yschimke
Copy link
Author

yschimke commented Apr 7, 2024

This current version is platform agnostic. So I suggest this is a separate task.

Also probably worth running the instrumentation tasks on a later Android emulator.

@yschimke
Copy link
Author

yschimke commented Apr 7, 2024

I was considering adding a shadow API to set a different cronet provider that would work for grpc and http3.

But I figured a follow up.

@yschimke
Copy link
Author

yschimke commented Apr 7, 2024

cc @fridek

@yschimke yschimke force-pushed the httpengine branch 3 times, most recently from d55ae62 to 4c5ce7e Compare April 7, 2024 10:09
Use the JavaCronetProvider from cronet-fallback.
Implement just enough to support URLConnection.
@utzcoz
Copy link
Member

utzcoz commented Apr 20, 2024

@hoisie @fridek, this PR looks great to me, what do you think about it?

@yschimke
Copy link
Author

@utzcoz anything I need to do on this, or just waiting for additional review?

@utzcoz
Copy link
Member

utzcoz commented Apr 27, 2024

@utzcoz anything I need to do on this, or just waiting for additional review?

It looks good to me. But it's better having @hoisie extra reviewing as it brings a new dependency to shadow framework.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants