1246763 - Refactor GMP info file parsing into helper class

Status: RESOLVED FIXED

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

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

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.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Refactor GMP info file parsing into helper class

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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?

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

(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.

Comment 4

[Pulsebot]

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

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

(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!(?)

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f00cf14ee294