1437893 - cdm-test-decryptor.cpp: Remove an unnecessary string initialization

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P3)

Description

[Sylvestre Ledru [:Sylvestre]]

https://dxr.mozilla.org/mozilla-central/source/dom/media/fake-cdm/cdm-test-decryptor.cpp?q=cdm-test-decryptor.cpp&redirect_type=direct#429

Found by http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-string-init.html
(only occurrence left in our code base).

The checker will be enabled at review phase.

A trivial bug, marking it as a good first bug to learn about our workflows.

Comment 1

[mack7121]

I'd like to take this on as a first bug.

Comment 2

[gaurangbansal18]

I would like to work on this bug as my first task.

Comment 3

[Ralph Giles (:rillian)]

If you're interested in working on this bug, please assign it to yourself and post a patch for comments. Whoever takes it first can have a first go, but feel free to work together.

Comment 4

[Videet Singhai]

Can anyone tell me -
how to clone this repository?
how to submit a patch?

I am a beginner.
Thank you

Comment 5

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

If you're interested in getting started on Firefox development here is a good place to start: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build

Comment 6

[Chinmay]

Can someone tell me how I can run the test to check if this bug has been fixed? I ran the './mach xpcshell-test services/common/tests/unit/' but it fails 1 test even without my changes.

(newbie here)

Comment 7

[Sylvestre Ledru [:Sylvestre]]

I don't think you need to run the tests, this should be transparent!

Comment 8

[Videet Singhai]

I am done with the changes....can anyone tell me how to submit it as a patch?
Thank you

Comment 9

[Sylvestre Ledru [:Sylvestre]]

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Submitting_the_patch

Comment 10

[Videet Singhai]

Sir, can u please list the steps for a first patch contribution, there are lots of documentation but I am finding them a bit confusing. It will be of great help. Thank you

Comment 11

[Sylvestre Ledru [:Sylvestre]]

The exercise here is exactly about that: learning how to read documentations and use it.
Just use the Bugzilla method explained in the link from comment #9. It should be enough!

Comment 12

[Venkatesh Prabhu :vprabhu]

Attachment: Bug 1437893 - Removed an unnecessary string initialization in cdm-test-decryptor.cpp

Review commit: https://reviewboard.mozilla.org/r/223442/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223442/

Comment 13

[Venkatesh Prabhu :vprabhu]

(In reply to Venkatesh Prabhu :vprabhu from comment #12)
> Created attachment 8954339 [details]
> Bug 1437893 - Removed an unnecessary string initialization in
> cdm-test-decryptor.cpp
> 
> Review commit: https://reviewboard.mozilla.org/r/223442/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/223442/

I have made the change. Please let me know if that is what was needed.

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8954339 [details]
Bug 1437893 - Removed an unnecessary string initialization in cdm-test-decryptor.cpp

https://reviewboard.mozilla.org/r/223442/#review229540

::: dom/media/fake-cdm/cdm-test-decryptor.cpp:429
(Diff revision 1)
>    ShutdownTimeout,
>    ShutdownStoreToken
>  };
>  
>  static ShutdownMode sShutdownMode = ShutdownNormal;
> -static string sShutdownToken = "";
> +static string sShutdownToken ;

No spaces before the ";"

Comment 15

[Venkatesh Prabhu :vprabhu]

Comment on attachment 8954339 [details]
Bug 1437893 - Removed an unnecessary string initialization in cdm-test-decryptor.cpp

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223442/diff/1-2/

Comment 16

[Venkatesh Prabhu :vprabhu]

(In reply to Sylvestre Ledru [:sylvestre] from comment #14)
> Comment on attachment 8954339 [details]
> Bug 1437893 - Removed an unnecessary string initialization in
> cdm-test-decryptor.cpp
> 
> https://reviewboard.mozilla.org/r/223442/#review229540
> 
> ::: dom/media/fake-cdm/cdm-test-decryptor.cpp:429
> (Diff revision 1)
> >    ShutdownTimeout,
> >    ShutdownStoreToken
> >  };
> >  
> >  static ShutdownMode sShutdownMode = ShutdownNormal;
> > -static string sShutdownToken = "";
> > +static string sShutdownToken ;
> 
> No spaces before the ";"

Done. Sorry about that. I thought of that after pushing the changes for review.

Comment 17

[Sylvestre Ledru [:Sylvestre]]

No worries! 
Now, ask for a review to cpearce (the code owner)

Comment 18

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8954339 [details]
Bug 1437893 - Removed an unnecessary string initialization in cdm-test-decryptor.cpp

https://reviewboard.mozilla.org/r/223442/#review229674

Thank you for the patch!

Comment 19

[Chris Pearce [:cpearce (Not reading bugmail)]]

Venkatesh: If you can mark the issue about the extra space that Sylvestre pointed out as resolved in MozReview, our patch landing system can push this code to our main repository.

Comment 20

[Venkatesh Prabhu :vprabhu]

(In reply to Chris Pearce (:cpearce) from comment #19)
> Venkatesh: If you can mark the issue about the extra space that Sylvestre
> pointed out as resolved in MozReview, our patch landing system can push this
> code to our main repository.

Done.

Comment 21

[Sylvestre Ledru [:Sylvestre]]

And I just landed it for you. Should be in Firefox 60.
Bravo!

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ce5b5962798
Removed an unnecessary string initialization in cdm-test-decryptor.cpp r=cpearce

Comment 23

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/7ce5b5962798