New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 874542 link

Starred by 1 user
Status: Fixed
Owner:
mef@chromium.org
Closed: Sep 18
Cc:
mef@chromium.org
█ yuriystelmakh@google.com
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Mac , Fuchsia
Pri: 3
Type: Feature

Blocking:
issue 786559



Sign in to add a comment

[Cronet] Cronet native API should provide grpc_support/bidirectional_stream interface on all supported platforms

Project Member Reported by mef@chromium.org, Aug 15 
Cronet native API is cross platform.

Components/grpc_support is also cross platform, although it is only used by Cronet on iOS.

We should expose bidirectional stream from Cronet on all supported platforms.

Use cases:

- Unit testing of gRPC on desktops (IIUIC that requires Linux or Mac OS X support).
- Use of native grpc on Android.
- Use of gRPC on Windows and OSX.

Comment 1 by mef@chromium.org, Aug 15

Blocking: 786559

Comment 2 by mef@chromium.org, Aug 16

Owner: mef@chromium.org
Status: Started (was: Available)
I've created https://chromium-review.googlesource.com/c/chromium/src/+/1178666 but it makes some bots, mostly windows and fuchsia, unhappy.
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 4 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec

commit 4327cf6c9069208ad77c28a8f8df3b71ff74f5ec
Author: Misha Efimov <mef@chromium.org>
Date: Tue Sep 04 16:35:46 2018

[Cronet] Provide bidirectional stream support from native CronetEngine.

Bug:  874542 
Change-Id: I8414e75049ee219f1c7491caf91f6282c42f23dd
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;master.tryserver.chromium.android:android_cronet_tester
Reviewed-on: https://chromium-review.googlesource.com/1178666
Commit-Queue: Misha Efimov <mef@chromium.org>
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588541}
[modify] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/cronet/cronet_url_request_context.cc
[modify] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/cronet/cronet_url_request_context.h
[modify] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/cronet/ios/BUILD.gn
[modify] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/cronet/ios/test/BUILD.gn
[modify] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/cronet/native/BUILD.gn
[modify] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/cronet/native/engine.cc
[modify] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/cronet/native/engine.h
[modify] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/cronet/native/include/cronet_c.h
[modify] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/cronet/native/test/BUILD.gn
[modify] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/cronet/native/test/engine_test.cc
[add] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/cronet/native/test/test_stream_engine.cc
[modify] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/cronet/native/test/test_util.cc
[modify] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/grpc_support/BUILD.gn
[modify] https://crrev.com/4327cf6c9069208ad77c28a8f8df3b71ff74f5ec/components/grpc_support/include/bidirectional_stream_c.h
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 5 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/83e7fc49ef7d1701dc8657dd98847ee5cb416038

commit 83e7fc49ef7d1701dc8657dd98847ee5cb416038
Author: Samuel Huang <huangs@chromium.org>
Date: Tue Sep 04 23:18:28 2018

Revert "[Cronet] Provide bidirectional stream support from native CronetEngine."

This reverts commit 4327cf6c9069208ad77c28a8f8df3b71ff74f5ec.

Reason for revert:  http://crbug.com/880618 : cronet_tests failing on chromium.memory/Linux CFI

Original change's description:
> [Cronet] Provide bidirectional stream support from native CronetEngine.
> 
> Bug:  874542 
> Change-Id: I8414e75049ee219f1c7491caf91f6282c42f23dd
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;master.tryserver.chromium.android:android_cronet_tester
> Reviewed-on: https://chromium-review.googlesource.com/1178666
> Commit-Queue: Misha Efimov <mef@chromium.org>
> Reviewed-by: Paul Jensen <pauljensen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#588541}

TBR=pauljensen@chromium.org,mef@chromium.org

Change-Id: I2e300b34b04a5a16ff3c2959bc7ae8ebfdf32b4c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  874542 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;master.tryserver.chromium.android:android_cronet_tester
Reviewed-on: https://chromium-review.googlesource.com/1205770
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588689}
[modify] https://crrev.com/83e7fc49ef7d1701dc8657dd98847ee5cb416038/components/cronet/cronet_url_request_context.cc
[modify] https://crrev.com/83e7fc49ef7d1701dc8657dd98847ee5cb416038/components/cronet/cronet_url_request_context.h
[modify] https://crrev.com/83e7fc49ef7d1701dc8657dd98847ee5cb416038/components/cronet/ios/BUILD.gn
[modify] https://crrev.com/83e7fc49ef7d1701dc8657dd98847ee5cb416038/components/cronet/ios/test/BUILD.gn
[modify] https://crrev.com/83e7fc49ef7d1701dc8657dd98847ee5cb416038/components/cronet/native/BUILD.gn
[modify] https://crrev.com/83e7fc49ef7d1701dc8657dd98847ee5cb416038/components/cronet/native/engine.cc
[modify] https://crrev.com/83e7fc49ef7d1701dc8657dd98847ee5cb416038/components/cronet/native/engine.h
[modify] https://crrev.com/83e7fc49ef7d1701dc8657dd98847ee5cb416038/components/cronet/native/include/cronet_c.h
[modify] https://crrev.com/83e7fc49ef7d1701dc8657dd98847ee5cb416038/components/cronet/native/test/BUILD.gn
[modify] https://crrev.com/83e7fc49ef7d1701dc8657dd98847ee5cb416038/components/cronet/native/test/engine_test.cc
[delete] https://crrev.com/809a84583521717d12c1e741257fd5ee77ba3fce/components/cronet/native/test/test_stream_engine.cc
[modify] https://crrev.com/83e7fc49ef7d1701dc8657dd98847ee5cb416038/components/cronet/native/test/test_util.cc
[modify] https://crrev.com/83e7fc49ef7d1701dc8657dd98847ee5cb416038/components/grpc_support/BUILD.gn
[modify] https://crrev.com/83e7fc49ef7d1701dc8657dd98847ee5cb416038/components/grpc_support/include/bidirectional_stream_c.h
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 5 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e838a3024b4cad151254cb53d3c070975f238f2c

commit e838a3024b4cad151254cb53d3c070975f238f2c
Author: Misha Efimov <mef@chromium.org>
Date: Wed Sep 05 19:30:55 2018

Reland "[Cronet] Provide bidirectional stream support from native CronetEngine."

This is a reland of 4327cf6c9069208ad77c28a8f8df3b71ff74f5ec

Original change's description:
> [Cronet] Provide bidirectional stream support from native CronetEngine.
> 
> Bug:  874542 
> Change-Id: I8414e75049ee219f1c7491caf91f6282c42f23dd
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;master.tryserver.chromium.android:android_cronet_tester
> Reviewed-on: https://chromium-review.googlesource.com/1178666
> Commit-Queue: Misha Efimov <mef@chromium.org>
> Reviewed-by: Paul Jensen <pauljensen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#588541}

Bug:  874542 
Change-Id: I1380e751fa00fae546c563d72da829b089458fe5
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;master.tryserver.chromium.android:android_cronet_tester
Reviewed-on: https://chromium-review.googlesource.com/1207613
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588967}
[modify] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/cronet/cronet_url_request_context.cc
[modify] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/cronet/cronet_url_request_context.h
[modify] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/cronet/ios/BUILD.gn
[modify] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/cronet/ios/test/BUILD.gn
[modify] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/cronet/native/BUILD.gn
[modify] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/cronet/native/engine.cc
[modify] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/cronet/native/engine.h
[modify] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/cronet/native/include/cronet_c.h
[modify] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/cronet/native/test/BUILD.gn
[modify] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/cronet/native/test/engine_test.cc
[add] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/cronet/native/test/test_stream_engine.cc
[modify] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/cronet/native/test/test_util.cc
[modify] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/grpc_support/BUILD.gn
[modify] https://crrev.com/e838a3024b4cad151254cb53d3c070975f238f2c/components/grpc_support/include/bidirectional_stream_c.h

Comment 6 by pauljensen@chromium.org, Sep 18 

@mef: is this done?

Comment 7 by mef@chromium.org, Sep 18

Status: Fixed (was: Started)
Yeah, the reland has succeeded. 

There are further improvements that could be done to bring bidirectional stream api in line with cronet native api, but those can be done separately with lower priority.
►

Sign in to add a comment