Closed Bug 1246763 Opened 5 years ago Closed 5 years ago

Refactor GMP info file parsing into helper class

Categories

(Core :: Audio/Video: GMP, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

Details

Attachments

(1 file)

Dealing with GMP info files would be easier if we factored the parsing of the info file and storing parsed results into a helper class. Then we'd not need to re-parse the entire file again when we want to read one field from the .info file.
Priority: -- → P2
Comment on attachment 8717134 [details] [diff] [review]
Refactor GMP info file parsing into helper class

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

r+ with question:

::: dom/media/gmp/GMPChild.cpp
@@ +325,5 @@
>      return false;
>    }
>  
> +  nsTArray<nsCString> libs;
> +  SplitAt(", ", mInfoParser.Get(NS_LITERAL_CSTRING("libraries")), libs);

You're now splitting the "libraries" list at spaces (not just commas) -- Is that really what you intend?
Attachment #8717134 - Flags: review?(gsquelart) → review+
(In reply to Gerald Squelart [:gerald] from comment #2)
> Comment on attachment 8717134 [details] [diff] [review]
> Refactor GMP info file parsing into helper class
> 
> Review of attachment 8717134 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ with question:
> 
> ::: dom/media/gmp/GMPChild.cpp
> @@ +325,5 @@
> >      return false;
> >    }
> >  
> > +  nsTArray<nsCString> libs;
> > +  SplitAt(", ", mInfoParser.Get(NS_LITERAL_CSTRING("libraries")), libs);
> 
> You're now splitting the "libraries" list at spaces (not just commas) -- Is
> that really what you intend?

Yes, that's intentional. This enabled me to remove the lib.Trim(" ") call.
(In reply to Chris Pearce (:cpearce) from comment #3)
> (In reply to Gerald Squelart [:gerald] from comment #2)
> > > +  SplitAt(", ", mInfoParser.Get(NS_LITERAL_CSTRING("libraries")), libs);
> > You're now splitting the "libraries" list at spaces (not just commas) -- Is
> > that really what you intend?
> 
> Yes, that's intentional. This enabled me to remove the lib.Trim(" ") call.

I see. Hopefully there are no libraries with spaces in their name!(?)
https://hg.mozilla.org/mozilla-central/rev/f00cf14ee294
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.