Closed Bug 1127656 Opened 9 years ago Closed 9 years ago

Remove class inheritance hierarchy from gonk PlatformDeocdeModule

Categories

(Core :: Audio/Video, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: ayang, Assigned: ayang)

Details

Attachments

(1 file, 3 obsolete files)

Attached patch remove_class_hierarchy (obsolete) — Splinter Review
Assignee: nobody → ayang
Attached patch remove_class_hierarchy (obsolete) — Splinter Review
Attachment #8597850 - Attachment is obsolete: true
Attachment #8597852 - Flags: review?(ajones)
Attached patch remove_class_hierarchy (obsolete) — Splinter Review
Attachment #8597852 - Attachment is obsolete: true
Attachment #8597852 - Flags: review?(ajones)
Attachment #8597927 - Flags: review?(ajones)
Comment on attachment 8597927 [details] [diff] [review]
remove_class_hierarchy

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

Sorry for the delay.

::: dom/media/fmp4/gonk/GonkAudioDecoderManager.cpp
@@ +116,5 @@
> +{
> +  MonitorAutoLock mon(mMonitor);
> +  nsRefPtr<MediaRawData> sample;
> +
> +  if (!aSample) {

Suggestion: it can be easier to read without using the not (!) as in |if (aSample) { .. } else { ... }| rather than |if (!aSample) { ... } else { ... }|
Attachment #8597927 - Flags: review?(ajones) → review+
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #4)
> Comment on attachment 8597927 [details] [diff] [review]
> remove_class_hierarchy
> 
> Review of attachment 8597927 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Sorry for the delay.
> 
> ::: dom/media/fmp4/gonk/GonkAudioDecoderManager.cpp
> @@ +116,5 @@
> > +{
> > +  MonitorAutoLock mon(mMonitor);
> > +  nsRefPtr<MediaRawData> sample;
> > +
> > +  if (!aSample) {
> 
> Suggestion: it can be easier to read without using the not (!) as in |if
> (aSample) { .. } else { ... }| rather than |if (!aSample) { ... } else { ...
> }|

As your wish.

Thank you for review.
Attachment #8601250 - Attachment is patch: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b41ebad8dbe7
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.