Fix promise resolution of FontFaceSet#load()

Fix promise resolution of FontFaceSet#load()

This patch changes the promise resolution logic of FontFaceSet#load()
to match the spec [1], in the following ways:

1. Rejects the promise with SyntaxError if the font argument fails to
   parse.
2. Otherwise, the promise should be resolved in the same way as
   Promise.all() called with all of the results of FontFace#load() of
   the matching font faces, i.e.
2.1. Resolves the promise with an array of the matching font faces when
     all the fonts are loaded, or
2.2. Rejects the promise with the rejection value of the first failed
     font face.

The new ScriptPromiseResolver Vector overloads need NoInline version of
v8Array, since existing v8Array() uses toV8 inside and hence requires
generated headers (see crbug.com/321569 for details of a similar issue).

[1] http://dev.w3.org/csswg/css-font-loading/#font-face-set-load

TEST=fast/css/fontfaceset-load.html
BUG=53213
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169611

[Kunihiko Sakamoto, 2014-03-19 08:12:11]

dglazkov@ for css/
haraken@ for bindings/

Could you take a look?

[haraken, 2014-03-19 08:31:57]

https://codereview.chromium.org/196383035/diff/20001/Source/bindings/v8/V8Bin...
File Source/bindings/v8/V8Binding.h (right):

https://codereview.chromium.org/196383035/diff/20001/Source/bindings/v8/V8Bin...
Source/bindings/v8/V8Binding.h:296: v8::Handle<v8::Value> v8ArrayNoInline(const
Vector<T, inlineCapacity>& iterator, v8::Isolate* isolate)

I don't fully understand why you call this v8Array"NoInline". It's inlined.

Doesn't normal v8Array() work for you?

[yhirano, 2014-03-19 09:09:57]

lgtm for ScriptPromiseResolver while I'm not sure whether you should implement
the HeapVector overload.

[Kunihiko Sakamoto, 2014-03-19 09:12:17]

https://codereview.chromium.org/196383035/diff/20001/Source/bindings/v8/V8Bin...
File Source/bindings/v8/V8Binding.h (right):

https://codereview.chromium.org/196383035/diff/20001/Source/bindings/v8/V8Bin...
Source/bindings/v8/V8Binding.h:296: v8::Handle<v8::Value> v8ArrayNoInline(const
Vector<T, inlineCapacity>& iterator, v8::Isolate* isolate)
On 2014/03/19 08:31:58, haraken wrote:
> 
> I don't fully understand why you call this v8Array"NoInline". It's inlined.
> 
> Doesn't normal v8Array() work for you?

Because this uses toV8NoInline(T) instead of toV8(T).
This function itself is inlined indeed ^^; any suggestions for naming?

Normal v8Array doesn't work. This code is used from FontFaceSet.cpp, and
toV8(FontFace) is defined in generated V8FontFace.h. We don't want to include
generated headers from Source/core. (see discussions in crbug.com/321569)

[haraken, 2014-03-19 09:33:40]

LGTM for bindings/.

I'm not really happy about the toV8Inline family but I cannot come up with
something better :-/

[dglazkov, 2014-03-19 15:42:34]

lgtm. I made naming suggestions, but feel free to ignore them.

https://codereview.chromium.org/196383035/diff/20001/Source/core/css/FontFace...
File Source/core/css/FontFace.cpp (right):

https://codereview.chromium.org/196383035/diff/20001/Source/core/css/FontFace...
Source/core/css/FontFace.cpp:395: ScriptPromise FontFace::load(ExecutionContext*
context)
This is the actual Promise FontFace.load. Maybe it should be loadWithPromise?

https://codereview.chromium.org/196383035/diff/20001/Source/core/css/FontFace...
Source/core/css/FontFace.cpp:419: void FontFace::loadInternal(ExecutionContext*
context)
This is the actual "load"

https://codereview.chromium.org/196383035/diff/20001/Source/core/css/FontFace.h
File Source/core/css/FontFace.h (right):

https://codereview.chromium.org/196383035/diff/20001/Source/core/css/FontFace...
Source/core/css/FontFace.h:102: void load(PassRefPtr<LoadFontCallback>,
ExecutionContext*);
"Load" overload :)

You even have loadInternal now. I wonder if we could name them a bit more
specifically so that there's less confusion for future us looking at this code.

[Kunihiko Sakamoto, 2014-03-20 01:06:31]

Thanks for the suggestions!

https://codereview.chromium.org/196383035/diff/20001/Source/core/css/FontFace...
File Source/core/css/FontFace.cpp (right):

https://codereview.chromium.org/196383035/diff/20001/Source/core/css/FontFace...
Source/core/css/FontFace.cpp:395: ScriptPromise FontFace::load(ExecutionContext*
context)
On 2014/03/19 15:42:34, dglazkov wrote:
> This is the actual Promise FontFace.load. Maybe it should be loadWithPromise?

This is a method of the FontFace interface, so it'd be good to keep it
unchanged.

https://codereview.chromium.org/196383035/diff/20001/Source/core/css/FontFace.h
File Source/core/css/FontFace.h (right):

https://codereview.chromium.org/196383035/diff/20001/Source/core/css/FontFace...
Source/core/css/FontFace.h:102: void load(PassRefPtr<LoadFontCallback>,
ExecutionContext*);
On 2014/03/19 15:42:34, dglazkov wrote:
> "Load" overload :)
> 
> You even have loadInternal now. I wonder if we could name them a bit more
> specifically so that there's less confusion for future us looking at this
code.
> 

Renamed to loadWithCallback.

[Kunihiko Sakamoto, 2014-03-20 01:08:25]

The CQ bit was checked by ksakamoto@chromium.org

[commit-bot: I haz the power, 2014-03-20 01:08:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ksakamoto@chromium.org/196383035/40001

[commit-bot: I haz the power, 2014-03-20 01:59:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-20 01:59:15]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[Kunihiko Sakamoto, 2014-03-20 02:29:15]

The CQ bit was checked by ksakamoto@chromium.org

[commit-bot: I haz the power, 2014-03-20 02:29:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ksakamoto@chromium.org/196383035/40001

[commit-bot: I haz the power, 2014-03-20 04:04:19]

Change committed as 169611