1473217 - (clang-tidy-unused-raii) Fix and enable clang-tidy's misc-unused-raii warnings

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Chris Peterson [:cpeterson]]

clang-tidy's misc-unused-raii check finds temporary objects that look like RAII objects:

https://clang.llvm.org/extra/clang-tidy/checks/bugprone-unused-raii.html

The canonical example for this is a scoped lock object missing a name:

  {
    scoped_lock(&global_mutex);
    critical_section();
  }

The destructor of the scoped_lock is called before the critical_section is entered, leaving it unprotected.

Comment 1

[Chris Peterson [:cpeterson]]

Attachment: Bug 1473217 - Part 1: webspeech: Fix clang-tidy's misc-unused-raii warning.

AutoNoJSAPI is an RAII class to put the JS engine in an unusable state and set the subject principal to System. Without a name, this AutoNoJSAPI is a temporary object that is destructed before GetUserMedia() is called, thus making the AutoNoJSAPI a no-op.

This AutoNoJSAPI was added in tab mirroring bug 1088758, with comments from you in bug 1088758 comment 5.

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

Comment 2

[Chris Peterson [:cpeterson]]

Attachment: Bug 1473217 - Part 2: dom: Fix clang-tidy's misc-unused-raii warnings.

nsPrintfCString creates a formatted string, but doesn't print it.

ExtractOriginData is called for its constructor's scope and origin out parameters. This ExtractOriginData object doesn't really need a name because it is unused after we have scope and origin,  but this change makes clang-tidy happy and makes the code clear that ExtractOriginData is an object and not a funcion being called.

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

Comment 3

[Chris Peterson [:cpeterson]]

Attachment: Bug 1473217 - Part 3: Enable clang-tidy's misc-unused-raii warnings.

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

Comment 4

[Chris Peterson [:cpeterson]]

Comment on attachment 8989833 [details]
Bug 1473217 - Part 3: Enable clang-tidy's misc-unused-raii warnings.

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

Comment 5

[Andi [:andi]]

thank you for the patch, let's make sure we don't regress: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9d00ee276ad624a0b06741afa6ad3485a3e288d8

Comment 6

[Andi [:andi]]

Comment on attachment 8989833 [details]
Bug 1473217 - Part 3: Enable clang-tidy's misc-unused-raii warnings.

https://reviewboard.mozilla.org/r/254794/#review261758

Comment 7

[Andrea Marchesini [:baku]]

Comment on attachment 8989832 [details]
Bug 1473217 - Part 2: dom: Fix clang-tidy's misc-unused-raii warnings.

https://reviewboard.mozilla.org/r/254792/#review262776

Comment 8

[Chris Peterson [:cpeterson]]

I will land patches 2 and 3 while waiting for patch 1 review.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bfb8b44ccc42
Part 2: dom: Fix misc-unused-raii warnings. r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb66954c173f
Part 3: clang-tidy: Enable misc-unused-raii check. r=andi

Comment 10

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/bfb8b44ccc42
https://hg.mozilla.org/mozilla-central/rev/cb66954c173f

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8989831 [details]
Bug 1473217 - Part 1: webspeech: Fix clang-tidy's misc-unused-raii warning.

Sorry, missed that.  Obvious fix

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/67f6872c7302
webspeech: Fix clang-tidy's misc-unused-raii warning. r=jesup

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/67f6872c7302