Bug 149807

Summary: [Intl] Change the return type of canonicalizeLocaleList() from JSArray* to Vector<String>
Product: WebKit Reporter: Sukolsak Sakshuwong <sukolsak>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: andy, benjamin, commit-queue, sukolsak
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Use the LHS Vector<String> instead of const Vector<String>& none

Description Sukolsak Sakshuwong 2015-10-05 08:18:11 PDT 
From ECMA-402, 9.2.1, the abstract operation CanonicalizeLocaleList returns a List of Strings. From the spec, we never modify the result from CanonicalizeLocaleList(). We never expose it to the user either. This patch changes the return type of canonicalizeLocaleList() from JSArray* to Vector<String>. This should ease the workload of the GC and make the code a bit easier to read.

http://www.ecma-international.org/ecma-402/1.0/ECMA-402.pdf
Comment 1 Sukolsak Sakshuwong 2015-10-05 08:26:49 PDT 
Created attachment 262439 [details]
Patch
Comment 2 Benjamin Poulain 2015-10-05 14:44:12 PDT 
Comment on attachment 262439 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=262439&action=review

Ok

> Source/JavaScriptCore/runtime/IntlCollatorConstructor.cpp:152
> +    const Vector<String>& requestedLocales = canonicalizeLocaleList(exec, exec->argument(0));

IMHO, using Vector<String> makes the lifetime clearer.
Comment 3 Sukolsak Sakshuwong 2015-10-05 15:05:24 PDT 
Created attachment 262467 [details]
Use the LHS Vector<String> instead of const Vector<String>&
Comment 4 Sukolsak Sakshuwong 2015-10-05 15:18:47 PDT 
(In reply to comment #2)
> Comment on attachment 262439 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=262439&action=review
> 
> Ok
> 
> > Source/JavaScriptCore/runtime/IntlCollatorConstructor.cpp:152
> > +    const Vector<String>& requestedLocales = canonicalizeLocaleList(exec, exec->argument(0));
> 
> IMHO, using Vector<String> makes the lifetime clearer.

Thanks! Fixed.
Comment 5 WebKit Commit Bot 2015-10-05 16:37:42 PDT 
Comment on attachment 262467 [details]
Use the LHS Vector<String> instead of const Vector<String>&

Clearing flags on attachment: 262467

Committed r190591: <http://trac.webkit.org/changeset/190591>
Comment 6 WebKit Commit Bot 2015-10-05 16:37:45 PDT 
All reviewed patches have been landed.  Closing bug.