[Cronet] Implement native UploadDataStream API. [chromium/src : master]

Misha Efimov (Gerrit)

unread,

Jul 26, 2018, 11:02:31 AM7/26/18

to ios-r...@chromium.org, net-r...@chromium.org, cbentze...@chromium.org, Liang M, Paul Jensen, Commit Bot, chromium...@chromium.org

Hi Paul,

I've refactored Cronet_UrlRequestImplto move |executor_| and |callback_| from inner "Callback" (I wonder whether we should rename it to "NetworkTasks" to outer Cronet_UrlRequestImpl.

This allows OnUploadDataProviderError() post error to the client.

I'll be happy to extract Cronet_UrlRequestImpl changes into separate CL to make it easier to review and reason about.

PTAL, and let me know.

thanks,
-m

View Change

4 comments:

File components/cronet/native/upload_data_sink.cc:
- Patch Set #15, Line 23: rk stack as an implementation of
  this class could use a comment mentioning: […]
  Done
- Patch Set #15, Line 74: r) {}
  should we add a network thread checker here and in CloseOnNetworkThread()?
  This one is NOT called on the network thread though and the CloseOnNetworkThread() is the only method on Cronet_UploadDataSinkImpl that is called on the network thread.
  I don't think there is a strong need to call CloseOnNetworkThread() from network thread, as it needs to post close to client executor anyway. Should it be named differently?
- Patch Set #15, Line 172:
  this access to upload_data_provider_ is done without lock_ being held; could it crash if CloseOnNetw […]
  I've changed CloseOnNetworkThread() to post clearing of |upload_data_provider_| to the |executor_| to prevent the race. WDYT?
File components/cronet/native/url_request.cc:
- Patch Set #16, Line 448: const std::string& proxy_server,
  Gotcha, thanks. […]
  Currently it is exposed only on iOS, and only supported for gRPC C library as it needs some TLC to make it generally useable.

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Liang M (Gerrit)

unread,

Jul 27, 2018, 2:03:56 PM7/27/18

to Misha Efimov, ios-r...@chromium.org, net-r...@chromium.org, cbentze...@chromium.org, Paul Jensen, Commit Bot, chromium...@chromium.org

View Change

1 comment:

File components/cronet/native/url_request.cc:

- Patch Set #16, Line 448: const std::string& proxy_server,

- Currently it is exposed only on iOS, and only supported for gRPC C library as it needs some TLC to m […]
  Done

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Paul Jensen (Gerrit)

unread,

Jul 30, 2018, 9:02:41 PM7/30/18

to Misha Efimov, ios-r...@chromium.org, net-r...@chromium.org, cbentze...@chromium.org, Liang M, Commit Bot, chromium...@chromium.org

The workings of the upload classes/threads has fallen out of my brain's working set in the intervening month (and vacation) so I'll need to grok it again. Hopefully tomorrow.

View Change

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Paul Jensen (Gerrit)

unread,

Jul 31, 2018, 3:02:14 PM7/31/18

to Misha Efimov, ios-r...@chromium.org, net-r...@chromium.org, cbentze...@chromium.org, Liang M, Commit Bot, chromium...@chromium.org

View Change

1 comment:

File components/cronet/native/generated/cronet.idl_c.h:
- Patch Set #21, Line 643:
  does this encourage use-after-frees? what is the lifetime/ownership of the returned raw string pointer? Is this documented somewhere that I'm forgetting?

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Misha Efimov (Gerrit)

unread,

Aug 1, 2018, 10:43:18 AM8/1/18

to ios-r...@chromium.org, net-r...@chromium.org, cbentze...@chromium.org, Paul Jensen, Liang M, Commit Bot, chromium...@chromium.org

View Change

1 comment:

File components/cronet/native/generated/cronet.idl_c.h:

- Patch Set #21, Line 643:

- does this encourage use-after-frees? what is the lifetime/ownership of the returned raw string poin […]
  That's an excellent question. The returned raw string pointer is backed by member variable of Error class, so it is valid until the error object is destroyed.
  We could introduce methods like Cronet_Error_message_copy() that will allocate a new string, but then we'll need to also provide methods to free strings.
  My thought is that it will be less critical once we have C++ wrapper library.
  There is no documentation short of design doc and code. :(

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Paul Jensen (Gerrit)

unread,

Aug 1, 2018, 3:02:24 PM8/1/18

to Misha Efimov, ios-r...@chromium.org, net-r...@chromium.org, cbentze...@chromium.org, Liang M, Commit Bot, chromium...@chromium.org

I'm still reviewing this.

View Change

8 comments:

File components/cronet/ios/test/cronet_http_test.mm:
- Patch Set #21, Line 328:
  is this change meant for this CL?
File components/cronet/native/generated/cronet.idl_c.h:

- Patch Set #21, Line 643:

- That's an excellent question. […]
  Hmm, seems like we cannot add a comment to this method either as it's generated. I'm not sure there's much we can do about this now. This was an issue before this CL. Marking resolved.
File components/cronet/native/io_buffer_with_cronet_buffer.h:
- Patch Set #21, Line 39: int
  int->size_t
File components/cronet/native/upload_data_sink.h:
- Patch Set #21, Line 62: Cronet_UrlRequestImpl* url_request_ = nullptr;
  nit: *->* const
- Patch Set #21, Line 65: Cronet_ExecutorPtr upload_data_provider_executor_ = nullptr;
  nit: Ptr->Ptr const
File components/cronet/native/upload_data_sink.cc:
- Patch Set #21, Line 49: Cronet_UploadDataSinkImpl* upload_data_sink_ = nullptr;
  nit: *->* const
- Patch Set #21, Line 53: Cronet_ExecutorPtr upload_data_provider_executor_ = nullptr;
  nit: Ptr->Ptr const
File components/cronet/native/url_request.cc:
- Patch Set #21, Line 188: *
  nit: *->* const

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Misha Efimov (Gerrit)

unread,

Aug 1, 2018, 5:44:05 PM8/1/18

to net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Paul Jensen, Liang M, Commit Bot, chromium...@chromium.org

Thanks for your comments, take your time!

View Change

8 comments:

File components/cronet/ios/test/cronet_http_test.mm:

- Patch Set #21, Line 328:
  is this change meant for this CL?

- No longer needed.
File components/cronet/native/generated/cronet.idl_c.h:

- Patch Set #21, Line 643:

- Hmm, seems like we cannot add a comment to this method either as it's generated. […]
  sg. We could add a comment to codegen template, but it is probably better to put together README / implementation notes. WDYT?
File components/cronet/native/io_buffer_with_cronet_buffer.h:

- Patch Set #21, Line 39: int
  int->size_t

- Interestingly //net uses int to express buffer length, case in point: IOBufferWithSize: https://cs.chromium.org/chromium/src/net/base/io_buffer.h?rcl=920c0ed0fda97855c5389b5fcc87163ff0d3c529&l=102
File components/cronet/native/upload_data_sink.h:
- Patch Set #21, Line 62: Cronet_UrlRequestImpl* const url_request_ = nullptr;
  nit: *->* const
  Done
- Patch Set #21, Line 65: Cronet_ExecutorPtr const upload_data_provider_executor_ = nullptr;
  nit: Ptr->Ptr const
  Done
File components/cronet/native/upload_data_sink.cc:
- Patch Set #21, Line 49: Cronet_UploadDataSinkImpl* const upload_data_sink_ = nullptr;
  nit: *->* const
  Done
- Patch Set #21, Line 53: Cronet_ExecutorPtr const upload_data_provider_executor_ = nullptr;
  nit: Ptr->Ptr const
  Done
File components/cronet/native/url_request.cc:

- Patch Set #21, Line 188: *
  nit: *->* const

- Done

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Paul Jensen (Gerrit)

unread,

Aug 2, 2018, 8:18:33 AM8/2/18

to Misha Efimov, net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Liang M, Commit Bot, chromium...@chromium.org

Still reviewing.

View Change

12 comments:

File components/cronet/native/generated/cronet.idl_c.h:

- Patch Set #21, Line 643:

- sg. […]
  If there's some way to get a comment to go from the idl to the generated header, that'd be great. Otherwise adding it to a doc is fine.
File components/cronet/native/io_buffer_with_cronet_buffer.h:

- Patch Set #21, Line 39: int

Interestingly //net uses int to express buffer length, case in point: IOBufferWithSize: https://cs. […]

Um note the comment right before the "int" constructor:
 // TODO(eroman): Deprecated. Use the size_t flavor instead. crbug.com/488553
I think we should use size_t here and wherever possible.

File components/cronet/native/test/test_upload_data_provider.h:
- Patch Set #22, Line 60:
  const
- Patch Set #22, Line 62:
  const
- Patch Set #22, Line 67:
  const
- Patch Set #22, Line 69:
  const
- Patch Set #22, Line 80:
  const
- Patch Set #22, Line 100:
  const
- Patch Set #22, Line 101:
  const

File components/cronet/native/test/test_upload_data_provider.cc:

Patch Set #22, Line 58:
unnecessary curly braces
Patch Set #22, Line 62:
unnecessary curly braces

Patch Set #22, Line 141:

  // Cronet_RunnablePtr runnable = Cronet_Runnable_CreateWith(nullptr);
  // Cronet_Runnable_SetClientContext(task.get();
  // |runnable| is passed to executor, which destroys it after execution.
  // Cronet_Executor_Execute(executor_, runnable);
  EXPECT_TRUE(executor_);
  std::move(task).Run();

hmm is this setup as a direct executor temporarily?

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Paul Jensen (Gerrit)

unread,

Aug 2, 2018, 1:58:51 PM8/2/18

to Misha Efimov, net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Liang M, Commit Bot, chromium...@chromium.org

View Change

4 comments:

File components/cronet/native/upload_data_sink.cc:

Patch Set #22, Line 106:

  // Only chunked upload can have the final chunk.
  CHECK(is_chunked_ || !final_chunk);
  // Read upload data length exceeds specified length.
  CHECK(bytes_read <= remaining_length_ || is_chunked_);
  remaining_length_ -= bytes_read;

Can we do something more like what I suggested in PS#15?  Instead of "|| is_chunked_" in each of these and setting remaining_length_ to some bogus value when chunked, can we do:
if (!is_chunked_) {
  // Only chunked upload can have the final chunk.
  CHECK(!final_chunk);
  // Read upload data length exceeds specified length.
  CHECK(bytes_read <= remaining_length_);
  remaining_length_ -= bytes_read;
}

Patch Set #22, Line 125:
```
  auto error = std::make_unique<Cronet_Error>();
  error->error_code = Cronet_Error_ERROR_CODE_ERROR_CALLBACK;
  error->message =
      base::StrCat({"Failure from UploadDataProvider: ", error_message});
```
Can you modify OnUploadDataProviderError() to take error_message, and construct the Cronet_Error itself? This will dedup the code here and in OnRewindError(), and url_request.cc also includes CreateCronet_Error() which can be used to more easily construct the Cronet_Error.

- Patch Set #22, Line 141:

- ```
  if (url_request_->IsDone())
    return;
```
  why do we check this here, but not in OnReadSucceeded or OnReadError()?
File components/cronet/native/url_request.cc:
- Patch Set #22, Line 374: error_ = std::move(error);
  could this overwrite an error from OnFailed?

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Misha Efimov (Gerrit)

unread,

Aug 2, 2018, 5:14:06 PM8/2/18

to net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Paul Jensen, Liang M, Commit Bot, chromium...@chromium.org

Paul, thanks! Have a nice week! :)

View Change

16 comments:

File components/cronet/native/generated/cronet.idl_c.h:

- Patch Set #21, Line 643:

- If there's some way to get a comment to go from the idl to the generated header, that'd be great. […]
  Ack
File components/cronet/native/io_buffer_with_cronet_buffer.h:
- Patch Set #21, Line 39: siz
  Um note the comment right before the "int" constructor: […]
  Done
File components/cronet/native/test/test_upload_data_provider.h:

- Patch Set #22, Line 60:
  const

- Done

- Patch Set #22, Line 62:
  const

- Done
- Patch Set #22, Line 67: const;
  const
  Done
- Patch Set #22, Line 69: const;
  const
  Done
- Patch Set #22, Line 80: const;
  const
  Done
- Patch Set #22, Line 100: SYNC;
  const
  Done
- Patch Set #22, Line 101: utor_;
  const
  Done
File components/cronet/native/test/test_upload_data_provider.cc:

- Patch Set #22, Line 58:
  unnecessary curly braces

- Done

- Patch Set #22, Line 62:
  unnecessary curly braces

- Done

- Patch Set #22, Line 141:

  // Cronet_RunnablePtr runnable = Cronet_Runnable_CreateWith(nullptr);


  // Cronet_Runnable_SetClientContext(task.get();
  // |runnable| is passed to executor, which destroys it after execution.
  // Cronet_Executor_Execute(executor_, runnable);
  EXPECT_TRUE(executor_);
  std::move(task).Run();

- hmm is this setup as a direct executor temporarily?

- Umh, yes, but I've forgotten about it.
  Thanks for reminding!
  I'll keep this comment open.
File components/cronet/native/upload_data_sink.cc:

- Patch Set #22, Line 106:

  // Bytes read exceeds buffer length.
  CHECK_LT(static_cast<size_t>(bytes_read), buffer_->io_buffer_len());


  if (!is_chunked_) {
    // Only chunked upload can have the final chunk.
    CHECK(!final_chunk);

- Can we do something more like what I suggested in PS#15? Instead of "|| is_chunked_" in each of the […]
  Done

- Patch Set #22, Line 125:

- ```
    if (!upload_data_provider_)
      return;
  }
  if (url_request_->IsDone())
```
  Can you modify OnUploadDataProviderError() to take error_message, and construct the Cronet_Error its […]
  Good idea! Done.

- Patch Set #22, Line 141:

- ```
  }
  remaining
```

- why do we check this here, but not in OnReadSucceeded or OnReadError()?

- Done
File components/cronet/native/url_request.cc:
- Patch Set #22, Line 374: // If |error_| is not null

- could this overwrite an error from OnFailed?

- Good catch! Done.

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Paul Jensen (Gerrit)

unread,

Aug 6, 2018, 2:47:17 PM8/6/18

to Misha Efimov, net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Liang M, Commit Bot, chromium...@chromium.org

View Change

2 comments:

File components/cronet/native/upload_data_sink.cc:
- Patch Set #15, Line 172:
  I've changed CloseOnNetworkThread() to post clearing of |upload_data_provider_| to the |executor_| t […]
  I don't think that prevents the race. |executor_| can be multi-threaded. Right now the CheckState() calls will abort so we avoid the race because of that, but as my other comment alludes to, I'm not sure we should be aborting.
File components/cronet/native/upload_data_sink.cc:
- Patch Set #23, Line 216:
  I think we need some delayed-close logic like the Java code has. Imagine an app decides to cancel a request for some reason (e.g. user hits cancel button), if they call Cronet_UrlRequest_Cancel() then we'll try and run this check that we're not in a callback, but I'm not sure there is a reason we won't be in a callback, or more annoyingly we could be half way through Cronet_UploadDataSinkImpl::Read() so "in_which_user_callback_ = READ" but we haven't called into Cronet_UploadDataProvider_Read() yet.

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Misha Efimov (Gerrit)

unread,

Aug 14, 2018, 5:49:46 PM8/14/18

to net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Tricium, Paul Jensen, Liang M, Commit Bot, chromium...@chromium.org

Thanks, PTAL.

View Change

5 comments:

File components/cronet/native/test/test_upload_data_provider.cc:

Patch Set #22, Line 141:

        upload_data_provider->read_pending_ = false;
        Cronet_UploadDataSink_OnReadSucceeded(upload_data_sink, bytes_read,
                                              final_chunk);
      },
      this, upload_data_sink, read.size(), final_chunk);

Umh, yes, but I've forgotten about it. […]

Done

File components/cronet/native/upload_data_sink.cc:
- Patch Set #15, Line 74: a_provider_executor),
  This one is NOT called on the network thread though and the CloseOnNetworkThread() is the only metho […]
  Renamed. WDYT?
- Patch Set #15, Line 172: nitializeUploadDataSt
  I don't think that prevents the race. |executor_| can be multi-threaded. […]
  I've changed Close() to postpone the close if we are in the callback. I've also changed Read() and Rewind() to copy |upload_data_provider_| to the local variable so it is valid outside the lock.
  WDYT?
- Patch Set #15, Line 187:
  ditto
  Ack
File components/cronet/native/upload_data_sink.cc:

- Patch Set #23, Line 216:

- I think we need some delayed-close logic like the Java code has. […]
  Done

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Paul Jensen (Gerrit)

unread,

Aug 15, 2018, 2:12:33 PM8/15/18

to Misha Efimov, net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Tricium, Liang M, Commit Bot, chromium...@chromium.org

Two questions:
1. Is it safe to assume callers of Cronet_UrlRequest_Destroy() know they can only call this once the Cronet_UrlRequest has entered a final state? Can we enforce this? I ask because without this guarantee it wouldn't be safe for Cronet_UrlRequestImpl::NetworkTasks methods to be accessing |url_request_|, correct?
2. When an app receives a Cronet_UploadDataProvider_Read() callback, and they're passed a Cronet_UploadDataSinkPtr, is it safe to assume they know they don't own it and shouldn't call Cronet_UploadDataSink_Destroy()? The Cronet_UploadDataSinkImpl is owned (and consequently destroyed) by the Cronet_UrlRequestImpl.

View Change

2 comments:

File components/cronet/native/test/test_upload_data_provider.h:
- Patch Set #25, Line 111: int
  int64_t?

File components/cronet/native/url_request.cc:

Patch Set #24, Line 203:

  base::AutoLock lock(lock_);
  // Request may already be destroyed if it hasn't started or got canceled.
  if (request_)
    request_->Destroy(false);

should we replace this with a call to DestroyRequestUnlessDone()?

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Paul Jensen (Gerrit)

unread,

Aug 15, 2018, 3:01:10 PM8/15/18

to Misha Efimov, net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Tricium, Liang M, Commit Bot, chromium...@chromium.org

View Change

2 comments:

File components/cronet/native/upload_data_sink.cc:
- Patch Set #15, Line 74: tor)
  Renamed. […]
  sg
- Patch Set #15, Line 172: _IN_CALLBACK;

- I've changed Close() to postpone the close if we are in the callback. […]

- I think that should work.

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Misha Efimov (Gerrit)

unread,

Aug 15, 2018, 3:27:54 PM8/15/18

to net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Paul Jensen, Tricium, Liang M, Commit Bot, chromium...@chromium.org

Patch Set 25:
(2 comments)

Two questions:
1. Is it safe to assume callers of Cronet_UrlRequest_Destroy() know they can only call this once the Cronet_UrlRequest has entered a final state? Can we enforce this? I ask because without this guarantee it wouldn't be safe for Cronet_UrlRequestImpl::NetworkTasks methods to be accessing |url_request_|, correct?

Yes, correct, with one exception of request that has never started would never enter the end state.

> 2. When an app receives a Cronet_UploadDataProvider_Read() callback, and they're passed a Cronet_UploadDataSinkPtr, is it safe to assume they know they don't own it and shouldn't call Cronet_UploadDataSink_Destroy()? The Cronet_UploadDataSinkImpl is owned (and consequently destroyed) by the Cronet_UrlRequestImpl.

Yes. I'll start writing implementation notes.

View Change

2 comments:

File components/cronet/native/test/test_upload_data_provider.h:

- Patch Set #25, Line 111: int
  int64_t?

- Oops, yes. I wonder why compiler didn't complain about set_bad_length().
File components/cronet/native/url_request.cc:

- Patch Set #24, Line 203:

  base::AutoLock lock(lock_);
  // Only request that has never started is allowed to exist at this point.
  // The app must wait for OnSucceeded / OnFailed / OnCanceled  callback before
  // destroying |this|.

- should we replace this with a call to DestroyRequestUnlessDone()?

- Done

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Paul Jensen (Gerrit)

unread,

Aug 17, 2018, 10:07:29 AM8/17/18

to Misha Efimov, net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Tricium, Liang M, Commit Bot, chromium...@chromium.org

Patch set 26:Code-Review +1

View Change

2 comments:

File components/cronet/native/upload_data_sink.cc:
- Patch Set #26, Line 46: / Callbacks posted to client executor from callback on the network thread.
  maybe reword this to an action, like: "Post |task| to client executor."
File components/cronet/native/url_request.cc:
- Patch Set #26, Line 142: NetworkTasks
  I think this is still called CronetURLRequest::Callback

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Misha Efimov (Gerrit)

unread,

Aug 20, 2018, 11:37:20 AM8/20/18

to net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Paul Jensen, Tricium, Liang M, Commit Bot, chromium...@chromium.org

Thanks for the review!

View Change

2 comments:

File components/cronet/native/upload_data_sink.cc:
- Patch Set #26, Line 46: / Post |task| to client executor.
  maybe reword this to an action, like: "Post |task| to client executor. […]
  Done
File components/cronet/native/url_request.cc:

- Patch Set #26, Line 142:

- I think this is still called CronetURLRequest::Callback
  Good catch, search-replace was too agressive.

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Misha Efimov (Gerrit)

unread,

Aug 20, 2018, 11:37:41 AM8/20/18

to net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Paul Jensen, Tricium, Liang M, Commit Bot, chromium...@chromium.org

Patch set 27:Commit-Queue +2

View Change

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Commit Bot (Gerrit)

unread,

Aug 20, 2018, 11:37:46 AM8/20/18

to Misha Efimov, net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Paul Jensen, Tricium, Liang M, chromium...@chromium.org

CQ is trying the patch.

Note: The patchset sent to CQ was uploaded after this CL was approved.
"Fix comments." https://chromium-review.googlesource.com/c/1086127/27

Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/1086127/27

Bot data: {"action": "start", "triggered_at": "2018-08-20T15:37:38.0Z", "cq_cfg_revision": "2d2bef7bb928a763e179d3e6e761d3c3c94c991d", "revision": "a9cb757ee964ea3e4b89dcfae6d463efdfea526d"}

View Change

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Commit Bot (Gerrit)

unread,

Aug 20, 2018, 12:08:53 PM8/20/18

to Misha Efimov, net-r...@chromium.org, ios-r...@chromium.org, cbentze...@chromium.org, Paul Jensen, Tricium, Liang M, chromium...@chromium.org

Commit Bot merged this change.

View Change

Approvals:
  Paul Jensen: Looks good to me
  Misha Efimov: Commit

[Cronet] Implement native UploadDataStream API.

Bug: 786559
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ied0c3de493c817e0836fcf42e09736509acbd2d1
Reviewed-on: https://chromium-review.googlesource.com/1086127
Commit-Queue: Misha Efimov <m...@chromium.org>
Reviewed-by: Paul Jensen <paulj...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584464}
---
M components/cronet/native/BUILD.gn
M components/cronet/native/buffer.cc
M components/cronet/native/cronet.idl
M components/cronet/native/generated/cronet.idl_c.h
M components/cronet/native/generated/cronet.idl_impl_interface.cc
M components/cronet/native/generated/cronet.idl_impl_interface.h
M components/cronet/native/generated/cronet.idl_impl_interface_unittest.cc
M components/cronet/native/io_buffer_with_cronet_buffer.cc
M components/cronet/native/io_buffer_with_cronet_buffer.h
M components/cronet/native/test/BUILD.gn
A components/cronet/native/test/test_upload_data_provider.cc
A components/cronet/native/test/test_upload_data_provider.h
M components/cronet/native/test/test_url_request_callback.cc
M components/cronet/native/test/test_url_request_callback.h
M components/cronet/native/test/url_request_test.cc
A components/cronet/native/upload_data_sink.cc
A components/cronet/native/upload_data_sink.h
M components/cronet/native/url_request.cc
M components/cronet/native/url_request.h
19 files changed, 1,493 insertions(+), 199 deletions(-)

To view, visit change 1086127. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward