Adding async handling for sending through the transport

ASSIGNED
Assigned to

Status

()

P3
normal
Rank:
25
ASSIGNED
6 years ago
a year ago

People

(Reporter: abr, Assigned: jesup)

Tracking

unspecified
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Reporter)

Comment 1

6 years ago
Created attachment 678158 [details] [diff] [review]
Adding async handling for sending through the transport
(Reporter)

Updated

6 years ago
Attachment #678158 - Flags: review?(rjesup)
(Reporter)

Updated

6 years ago
Attachment #678158 - Flags: review?(ekr)

Comment 2

6 years ago
Comment on attachment 678158 [details] [diff] [review]
Adding async handling for sending through the transport

Review of attachment 678158 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/mtransport/test/transport_unittests.cpp
@@ +289,4 @@
>    }
>  
> +  void SetAsync(bool f) { 
> +    send_async_ = f;

Remove trailing whitespace.
(Assignee)

Comment 3

6 years ago
Comment on attachment 678158 [details] [diff] [review]
Adding async handling for sending through the transport

Review of attachment 678158 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/mtransport/test/transport_unittests.cpp
@@ +276,5 @@
>  
>    TransportResult SendPacket(const unsigned char* data, size_t len) {
> +    if (send_async_) {
> +      return flow_->SendPacket(data, len, TF_ASYNC);
> +    } else {

Minor style: 
  if (foo) {
    return bar;
  }
  return xyz

@@ +278,5 @@
> +    if (send_async_) {
> +      return flow_->SendPacket(data, len, TF_ASYNC);
> +    } else {
> +      TransportResult ret;
> +      

trailing whitespace

::: media/mtransport/transportflow.h
@@ +53,5 @@
>    // at the time. This way you don't need to do top()->Foo().
>    TransportLayer::State state(); // Current state
> +
> +  // See transportlayer.h for flags
> +  TransportResult SendPacket(const unsigned char *data, size_t len, 

trailing space
Attachment #678158 - Flags: review?(rjesup) → review+

Comment 4

6 years ago
Comment on attachment 678158 [details] [diff] [review]
Adding async handling for sending through the transport

Review of attachment 678158 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm with changes below.

::: media/mtransport/test/transport_unittests.cpp
@@ +97,5 @@
>  class TransportTestPeer : public sigslot::has_slots<> {
>   public:
>    TransportTestPeer(nsCOMPtr<nsIEventTarget> target, std::string name)
> +      : send_async_(false),
> +        name_(name), target_(target),

These can all go on one line.

::: media/mtransport/transportlayer.cpp
@@ +64,5 @@
> +
> +  unsigned char *data_copy = new unsigned char[len];
> +  memcpy(data_copy, data, len);
> +
> +  target_->Dispatch(WrapRunnable(this, &TransportLayer::SendPacketInt, 

Please add a comment that we always return success.
Attachment #678158 - Flags: review?(ekr) → review+
(Reporter)

Updated

6 years ago
Assignee: nobody → adam
(Reporter)

Comment 5

6 years ago
Created attachment 678537 [details] [diff] [review]
Added flags field to TransportFlow. Defined and implemented a TF_ASYNC flag to indicate that data should be sent out asynchronously (iff we're not sending from the socket thread).
(Reporter)

Updated

6 years ago
Attachment #678158 - Attachment is obsolete: true
(Reporter)

Updated

6 years ago
Attachment #678537 - Flags: checkin?(rjesup)

Updated

6 years ago
Priority: -- → P2
Whiteboard: [WebRTC][blocking-webrtc-]
Randell, is this waiting on something to land?
(Assignee)

Comment 7

6 years ago
Comment on attachment 678537 [details] [diff] [review]
Added flags field to TransportFlow. Defined and implemented a TF_ASYNC flag to indicate that data should be sent out asynchronously (iff we're not sending from the socket thread).

Carrying forward r+ (myself and ekr)
Attachment #678537 - Flags: review+
(Reporter)

Updated

6 years ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 8

6 years ago
Comment on attachment 678537 [details] [diff] [review]
Added flags field to TransportFlow. Defined and implemented a TF_ASYNC flag to indicate that data should be sent out asynchronously (iff we're not sending from the socket thread).

Cancelling checkin? until I have code to make use of this feature in DataChannels, and reassigning the bug to myself to do so.
Attachment #678537 - Flags: checkin?(rjesup)
(Assignee)

Updated

6 years ago
Assignee: adam → rjesup

Updated

4 years ago
backlog: --- → webRTC+
Rank: 25
Whiteboard: [WebRTC][blocking-webrtc-]
Mass change P2->P3 to align with new Mozilla triage process.
Priority: P2 → P3
You need to log in before you can comment on or make changes to this bug.