Bug 40602 – Use an array for extensions in MimeClassInfo

• Component: New Bugs
• Status: RESOLVED FIXED

[Anders Carlsson]

Use an array for extensions in MimeClassInfo

[Anders Carlsson]

Created attachment 58737 [details]
Patch

[Anders Carlsson]

Please don't review this yet, I'd like to run it through the EWS bots.

[Darin Adler]

Comment on attachment 58737 [details]
Patch

> +    const Vector<String> &extensions = mimeClassInfo().extensions;

The "&" here is in the wrong place.

> +    StringBuilder builder;
> +    for (size_t i = 0; i < extensions.size(); ++i) {
> +        if (i)
> +            builder.append(',');
> +        builder.append(extensions[i]);
> +    }
> +    return builder.toString();

Would Vector<UChar> do a better job? I never know which to use.

> +            NSArray *extensions = [plugin extensionsForMIMEType:MIME];
> +            NSUInteger count = [extensions count];
> +            for (NSUInteger i = 0; i < count; ++i) {
> +                NSString *extension = [extensions objectAtIndex:i];
> +                mime.extensions.append(extension);
> +            }

This is a lot like toStringVector in WebView.mm and WebEditorClient::getGuessesForWord in WebEditorClient.mm. I wish we had shared code to make NSArray of NSString into Vector<String> and vice versa.

r=me

[Anders Carlsson]

(In reply to comment #3)
> (From update of attachment 58737 [details])
> > +    const Vector<String> &extensions = mimeClassInfo().extensions;
> 
> The "&" here is in the wrong place.
> 

Fixed.

> 
> Would Vector<UChar> do a better job? I never know which to use.

I don't think it matters; this code is not performance sensitive.

> This is a lot like toStringVector in WebView.mm and WebEditorClient::getGuessesForWord in WebEditorClient.mm. I wish we had shared code to make NSArray of NSString into Vector<String> and vice versa.

I agree. However, in this case I think that it would be better to convert WebKit over to using WebCore types for its plug-in database.

> 
> r=me

Thanks!

[Anders Carlsson]

Committed r61168: <http://trac.webkit.org/changeset/61168>

[WebKit Review Bot]

http://trac.webkit.org/changeset/61168 might have broken Chromium Linux Release