1163201 - Remove usage of |#ifdef PR_LOGGING| from various dom subdirectories

Status: RESOLVED FIXED

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

Description

[Eric Rahm [:erahm]]

In bug 1161238 we plan on removing |--disable-logging| which makes |#ifdef PR_LOGGING| redundant.

This covers:
- dom/camera/
- dom/html/
- dom/media/

Comment 1

[Eric Rahm [:erahm]]

Attachment: Part 1: Remove instances of #ifdef PR_LOGGING in dom/

PR_LOGGING is now always defined, we can remove #ifdefs checking for it.

Comment 2

[Eric Rahm [:erahm]]

Comment on attachment 8604365 [details] [diff] [review]
Part 1: Remove instances of #ifdef PR_LOGGING in dom/

Masayuki can you review the changes in |dom/events/IMEStateManager.cpp|. I want to double check that we really want to MOZ_CRASH in |IMEStateManager::OnDestroyPresContext|.

Comment 3

[Eric Rahm [:erahm]]

Attachment: Part 2: Wrap expensive calls in PR_LOG_TEST

Comment 4

[Eric Rahm [:erahm]]

Attachment: Part 3: Make mSamples private in |MediaEngineWebRTCAudioSource|

|mSamples| was made protected just to satisfy a warning if PR_LOGGING was not
set. Now that it is always defined this no longer is necessary.

Comment 5

[Eric Rahm [:erahm]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c81955a3582d

Comment 6

[Eric Rahm [:erahm]]

Comment on attachment 8604975 [details] [diff] [review]
Part 2: Wrap expensive calls in PR_LOG_TEST

gcp, can you review the change to |dom/media/systemservices/OpenSLESProvider.cpp|? This was fixing what presumably was a copy and paste error.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8604365 [details] [diff] [review]
Part 1: Remove instances of #ifdef PR_LOGGING in dom/

Perhaps, not necessary but I want. The MOZ_CRASH() *might* detect memory leak of very rare case.

Comment 8

[Chris Peterson [:cpeterson]]

Comment on attachment 8604976 [details] [diff] [review]
Part 3: Make mSamples private in |MediaEngineWebRTCAudioSource|

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

LGTM. Thanks for removing my workaround. :)

Comment 9

[Eric Rahm [:erahm]]

Attachment: Part 3: Remove mSamples in |MediaEngineWebRTCAudioSource|

|mSamples| was made protected just to satisfy a warning if PR_LOGGING was not
set. As it turns out the warning is correct and mSamples is not used, so lets
remove it.

Comment 10

[Eric Rahm [:erahm]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b0532c93bd1f

Comment 11

[Chris Peterson [:cpeterson]]

Comment on attachment 8605365 [details] [diff] [review]
Part 3: Remove mSamples in |MediaEngineWebRTCAudioSource|

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

LGTM. Thanks!

Comment 12

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9104ef257c4a
https://hg.mozilla.org/integration/mozilla-inbound/rev/b2e89be9c64d
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ed296e3c72e

Comment 13

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ec7cf8914e8e

Comment 14

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fc5813496d2f

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9104ef257c4a
https://hg.mozilla.org/mozilla-central/rev/b2e89be9c64d
https://hg.mozilla.org/mozilla-central/rev/9ed296e3c72e
https://hg.mozilla.org/mozilla-central/rev/ec7cf8914e8e
https://hg.mozilla.org/mozilla-central/rev/fc5813496d2f