Closed Bug 1251714 Opened 4 years ago Closed 4 years ago

use UniquePtr instead of ScopedDeletePtr in media/

Categories

(Core :: WebRTC, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed
Blocking Flags:

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(1 file, 1 obsolete file)

UniquePtr is more standard than ScopedDeletePtr; using standard
constructs whenever possible is preferable.

This patch merits a couple explanations:

- Where it made sense, I tried to convert:

    T* foo() {
      UniquePtr<T> x = ...;
      ...
      return x.release();
    }

  into:

    UniquePtr<T> foo()

  with corresponding changes inside |foo|'s body.

- The attentive reader will note that:

    auto x = MakeUnique<T>(...);

  is used sometimes and:

    UniquePtr<T> x(new T(...));

  is used sometimes.  I would prefer to use the former, but was stymied
  in several places due to protected constructors.  (MakeUnique doesn't
  have access to those protected constructors, natch.)
Attachment #8724179 - Flags: review?(rjesup)
Update with some other patches I had lying about to fix new/free mismatch
issues.
Attachment #8724800 - Flags: review?(rjesup)
Attachment #8724179 - Attachment is obsolete: true
Attachment #8724179 - Flags: review?(rjesup)
Comment on attachment 8724800 [details] [diff] [review]
use UniquePtr instead of ScopedDeletePtr in media/

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

::: media/mtransport/transportflow.cpp
@@ +34,5 @@
>    // is maintained, it shouldn't be possible to access and
>    // destroy it simultaneously. The conversion to an nsAutoPtr
>    // ensures automatic destruction of the queue at exit of
>    // DestroyFinal.
> +  nsAutoPtr<std::deque<TransportLayer*> > layers_tmp(layers_.release());

Since you're touching it, "> >" -> ">>"

::: media/mtransport/transportflow.h
@@ +134,5 @@
>    static void ClearLayers(std::queue<TransportLayer *>* layers);
>  
>    std::string id_;
>    TransportLayer::State state_;
> +  UniquePtr<std::deque<TransportLayer *> > layers_;

ditto

::: media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
@@ +512,5 @@
>      return;
>    }
>  
>    // Make a copy rather than cast away constness
> +  auto inner_data = MakeUnique<unsigned char[]>(len);

I presume this does the correct delete []
Attachment #8724800 - Flags: review?(rjesup) → review+
backlog: --- → webrtc/webaudio+
Rank: 25
Priority: -- → P2
Bleh.
Flags: needinfo?(nfroyd)
(In reply to Pulsebot from comment #8)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/181f18c5d1ad

I fixed the gtest issues by not using static UniquePtr instances; apparently some of the tests don't shut things down properly. =/

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a7c710ff0a6
https://hg.mozilla.org/mozilla-central/rev/181f18c5d1ad
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Duplicate of this bug: 1154499
You need to log in before you can comment on or make changes to this bug.