Closed Bug 1163201 Opened 5 years ago Closed 5 years ago

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

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox40 --- affected
firefox41 --- fixed

People

(Reporter: erahm, Assigned: erahm)

References

Details

Attachments

(3 files, 1 obsolete file)

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

This covers:
- dom/camera/
- dom/html/
- dom/media/
PR_LOGGING is now always defined, we can remove #ifdefs checking for it.
Attachment #8604365 - Flags: review?(nfroyd)
Assignee: nobody → erahm
Status: NEW → ASSIGNED
Attachment #8604365 - Flags: review?(nfroyd) → review+
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|.
Attachment #8604365 - Flags: review?(masayuki)
Blocks: 1164297
Attachment #8604975 - Flags: review?(nfroyd)
|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.
Attachment #8604976 - Flags: review?(cpeterson)
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.
Attachment #8604975 - Flags: review?(gpascutto)
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.
Attachment #8604365 - Flags: review?(masayuki) → review+
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. :)
Attachment #8604976 - Flags: review?(cpeterson) → review+
Attachment #8604975 - Flags: review?(gpascutto) → review+
Attachment #8604975 - Flags: review?(nfroyd) → review+
|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.
Attachment #8605365 - Flags: review?(cpeterson)
Attachment #8604976 - Attachment is obsolete: true
Comment on attachment 8605365 [details] [diff] [review]
Part 3: Remove mSamples in |MediaEngineWebRTCAudioSource|

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

LGTM. Thanks!
Attachment #8605365 - Flags: review?(cpeterson) → review+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.