rAc: make requestAutocomplete() return a promise.

rAc: make requestAutocomplete() return a promise.

See whatwg@ thread for more details:
http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2014-April/254142.html

R=esprehn@chromium.org,tkent@chromium.org
BUG=343630

[tkent, 2014-04-17 02:48:16]

yhirano, can you review Promise-releated code?

[tkent, 2014-04-17 02:51:40]

https://chromiumcodereview.appspot.com/228783007/diff/170001/Source/core/html...
File Source/core/html/HTMLFormElement.h (right):

https://chromiumcodereview.appspot.com/228783007/diff/170001/Source/core/html...
Source/core/html/HTMLFormElement.h:187:
WillBePersistentHeapVector<OwnPtrWillBeMember<AutocompleteRequest> >
m_finishedAutocompleteRequests;
This should be |Vector<OwnPtr<AutocompleteRequest>>| because
AutomcompleteRequest is not garbage-collected.

[yhirano, 2014-04-17 10:57:48]

https://codereview.chromium.org/228783007/diff/170001/LayoutTests/fast/forms/...
File LayoutTests/fast/forms/form-request-autocomplete.html (right):

https://codereview.chromium.org/228783007/diff/170001/LayoutTests/fast/forms/...
LayoutTests/fast/forms/form-request-autocomplete.html:24: checkPromiseRejects();
Can you serialize the tests using Promises?
Otherwise, the test could be broken with a subtle Promises internal execution
order change compliant to the spec.

https://codereview.chromium.org/228783007/diff/170001/Source/core/html/HTMLFo...
File Source/core/html/HTMLFormElement.cpp (right):

https://codereview.chromium.org/228783007/diff/170001/Source/core/html/HTMLFo...
Source/core/html/HTMLFormElement.cpp:127: NewScriptState::Scope
scope(m_scriptState.get());
Can you use ScriptPromiseResolverWithContext instead of a pair of
ScriptPromiseResolver and NewScriptState?
Currently it doesn't provide a way to resolve a promise with undefined outside a
V8 context, but https://codereview.chromium.org/238723009/ implements the
functionality, which should be landed soon.

[Dan Beam, 2014-04-17 23:02:03]

https://codereview.chromium.org/228783007/diff/170001/LayoutTests/fast/forms/...
File LayoutTests/fast/forms/form-request-autocomplete.html (right):

https://codereview.chromium.org/228783007/diff/170001/LayoutTests/fast/forms/...
LayoutTests/fast/forms/form-request-autocomplete.html:24: checkPromiseRejects();
On 2014/04/17 10:57:48, yhirano wrote:
> Can you serialize the tests using Promises?
> Otherwise, the test could be broken with a subtle Promises internal execution
> order change compliant to the spec.

Done. (order doesn't matter now)

https://codereview.chromium.org/228783007/diff/170001/Source/core/html/HTMLFo...
File Source/core/html/HTMLFormElement.cpp (right):

https://codereview.chromium.org/228783007/diff/170001/Source/core/html/HTMLFo...
Source/core/html/HTMLFormElement.cpp:127: NewScriptState::Scope
scope(m_scriptState.get());
On 2014/04/17 10:57:48, yhirano wrote:
> Can you use ScriptPromiseResolverWithContext instead of a pair of
> ScriptPromiseResolver and NewScriptState?
> Currently it doesn't provide a way to resolve a promise with undefined outside
a
> V8 context, but https://codereview.chromium.org/238723009/ implements the
> functionality, which should be landed soon.

Done. (patched in your CL locally)

https://codereview.chromium.org/228783007/diff/170001/Source/core/html/HTMLFo...
File Source/core/html/HTMLFormElement.h (right):

https://codereview.chromium.org/228783007/diff/170001/Source/core/html/HTMLFo...
Source/core/html/HTMLFormElement.h:187:
WillBePersistentHeapVector<OwnPtrWillBeMember<AutocompleteRequest> >
m_finishedAutocompleteRequests;
On 2014/04/17 02:51:41, tkent wrote:
> This should be |Vector<OwnPtr<AutocompleteRequest>>| because
> AutomcompleteRequest is not garbage-collected.

Done.

https://codereview.chromium.org/228783007/diff/220001/Source/core/html/HTMLFo...
File Source/core/html/HTMLFormElement.cpp (right):

https://codereview.chromium.org/228783007/diff/220001/Source/core/html/HTMLFo...
Source/core/html/HTMLFormElement.cpp:127: NewScriptState::Scope
scope(m_resolver->scriptState());
^ still had to create a script scope to create new v8 values here

[yhirano, 2014-04-18 07:32:33]

https://codereview.chromium.org/228783007/diff/220001/Source/core/html/HTMLFo...
File Source/core/html/HTMLFormElement.cpp (right):

https://codereview.chromium.org/228783007/diff/220001/Source/core/html/HTMLFo...
Source/core/html/HTMLFormElement.cpp:127: NewScriptState::Scope
scope(m_resolver->scriptState());
On 2014/04/17 23:02:04, Dan Beam wrote:
> ^ still had to create a script scope to create new v8 values here
When https://codereview.chromium.org/238723009/ lands, you can resolve a promise
like: m_resolver->resolve(V8UndefinedType()) without NewScriptScope. Can you
wait a while?
Sorry for the inconvenience.

[Dan Beam, 2014-04-18 21:33:07]

https://codereview.chromium.org/228783007/diff/220001/Source/core/html/HTMLFo...
File Source/core/html/HTMLFormElement.cpp (right):

https://codereview.chromium.org/228783007/diff/220001/Source/core/html/HTMLFo...
Source/core/html/HTMLFormElement.cpp:127: NewScriptState::Scope
scope(m_resolver->scriptState());
On 2014/04/18 07:32:33, yhirano wrote:
> On 2014/04/17 23:02:04, Dan Beam wrote:
> > ^ still had to create a script scope to create new v8 values here
> When https://codereview.chromium.org/238723009/ lands, you can resolve a
promise
> like: m_resolver->resolve(V8UndefinedType()) without NewScriptScope. Can you
> wait a while?
> Sorry for the inconvenience.

yeah, it's all good -- this CL isn't incredibly urgent.

[yhirano, 2014-04-21 09:31:31]

https://codereview.chromium.org/228783007/diff/220001/Source/core/html/HTMLFo...
File Source/core/html/HTMLFormElement.cpp (right):

https://codereview.chromium.org/228783007/diff/220001/Source/core/html/HTMLFo...
Source/core/html/HTMLFormElement.cpp:127: NewScriptState::Scope
scope(m_resolver->scriptState());
On 2014/04/18 21:33:07, Dan Beam wrote:
> On 2014/04/18 07:32:33, yhirano wrote:
> > On 2014/04/17 23:02:04, Dan Beam wrote:
> > > ^ still had to create a script scope to create new v8 values here
> > When https://codereview.chromium.org/238723009/ lands, you can resolve a
> promise
> > like: m_resolver->resolve(V8UndefinedType()) without NewScriptScope. Can you
> > wait a while?
> > Sorry for the inconvenience.
> 
> yeah, it's all good -- this CL isn't incredibly urgent.

You can use it now. Thanks!

[Dan Beam, 2014-04-22 00:43:53]

https://chromiumcodereview.appspot.com/228783007/diff/220001/Source/core/html...
File Source/core/html/HTMLFormElement.cpp (right):

https://chromiumcodereview.appspot.com/228783007/diff/220001/Source/core/html...
Source/core/html/HTMLFormElement.cpp:127: NewScriptState::Scope
scope(m_resolver->scriptState());
On 2014/04/21 09:31:32, yhirano wrote:
> On 2014/04/18 21:33:07, Dan Beam wrote:
> > On 2014/04/18 07:32:33, yhirano wrote:
> > > On 2014/04/17 23:02:04, Dan Beam wrote:
> > > > ^ still had to create a script scope to create new v8 values here
> > > When https://codereview.chromium.org/238723009/ lands, you can resolve a
> > promise
> > > like: m_resolver->resolve(V8UndefinedType()) without NewScriptScope. Can
you
> > > wait a while?
> > > Sorry for the inconvenience.
> > 
> > yeah, it's all good -- this CL isn't incredibly urgent.
> 
> You can use it now. Thanks!

merged in your change, ptal

[yhirano, 2014-04-22 02:03:36]

https://codereview.chromium.org/228783007/diff/270001/Source/core/html/HTMLFo...
File Source/core/html/HTMLFormElement.cpp (right):

https://codereview.chromium.org/228783007/diff/270001/Source/core/html/HTMLFo...
Source/core/html/HTMLFormElement.cpp:133: m_resolver->reject(ScriptValue(error,
isolate));
Please call resolve or reject out of the extent of NewScriptState::Scope.
The following code works:
 v8::HandleScope handleScope;
 v8::Local<v8::Value> error;
 {
   NewScriptState::Scope scope(m_resolver->scriptState());
   error = ...;
 }
 m_resolver->reject(error);

[Dan Beam, 2014-04-23 01:21:03]

https://codereview.chromium.org/228783007/diff/270001/Source/core/html/HTMLFo...
File Source/core/html/HTMLFormElement.cpp (right):

https://codereview.chromium.org/228783007/diff/270001/Source/core/html/HTMLFo...
Source/core/html/HTMLFormElement.cpp:133: m_resolver->reject(ScriptValue(error,
isolate));
On 2014/04/22 02:03:37, yhirano wrote:
> Please call resolve or reject out of the extent of NewScriptState::Scope.

Done.

> The following code works:
>  v8::HandleScope handleScope;
>  v8::Local<v8::Value> error;
>  {
>    NewScriptState::Scope scope(m_resolver->scriptState());
>    error = ...;
>  }
>  m_resolver->reject(error);
> 

This code didn't work for me :(, see updated.

[yhirano, 2014-04-23 05:13:16]

On 2014/04/23 01:21:03, Dan Beam wrote:
>
https://codereview.chromium.org/228783007/diff/270001/Source/core/html/HTMLFo...
> File Source/core/html/HTMLFormElement.cpp (right):
> 
>
https://codereview.chromium.org/228783007/diff/270001/Source/core/html/HTMLFo...
> Source/core/html/HTMLFormElement.cpp:133:
m_resolver->reject(ScriptValue(error,
> isolate));
> On 2014/04/22 02:03:37, yhirano wrote:
> > Please call resolve or reject out of the extent of NewScriptState::Scope.
> 
> Done.

Thanks! LGTM.

> 
> > The following code works:
> >  v8::HandleScope handleScope;
> >  v8::Local<v8::Value> error;
> >  {
> >    NewScriptState::Scope scope(m_resolver->scriptState());
> >    error = ...;
> >  }
> >  m_resolver->reject(error);
> > 
> 
> This code didn't work for me :(, see updated.
Sorry, I was wrong. Your solution looks reasonable.

[tkent, 2014-04-23 06:36:57]

https://codereview.chromium.org/228783007/diff/330001/Source/core/dom/Autocom...
File Source/core/dom/AutocompleteError.cpp (right):

https://codereview.chromium.org/228783007/diff/330001/Source/core/dom/Autocom...
Source/core/dom/AutocompleteError.cpp:1: /*
Ditto.

https://codereview.chromium.org/228783007/diff/330001/Source/core/dom/Autocom...
Source/core/dom/AutocompleteError.cpp:37: : DOMException(0, "AutocompleteError")
Is it allowed to create a DOMException with code=0?

https://codereview.chromium.org/228783007/diff/330001/Source/core/dom/Autocom...
File Source/core/dom/AutocompleteError.h (right):

https://codereview.chromium.org/228783007/diff/330001/Source/core/dom/Autocom...
Source/core/dom/AutocompleteError.h:1: /*
Please use the 3-line copyright header for new files.

https://codereview.chromium.org/228783007/diff/330001/Source/core/dom/Autocom...
File Source/core/dom/AutocompleteError.idl (right):

https://codereview.chromium.org/228783007/diff/330001/Source/core/dom/Autocom...
Source/core/dom/AutocompleteError.idl:31: interface AutocompleteError :
DOMException {
Inheriting DOMException looks very strange.

[tkent, 2014-04-23 06:38:40]

https://codereview.chromium.org/228783007/diff/330001/Source/core/dom/Autocom...
File Source/core/dom/AutocompleteError.cpp (right):

https://codereview.chromium.org/228783007/diff/330001/Source/core/dom/Autocom...
Source/core/dom/AutocompleteError.cpp:1: /*
On 2014/04/23 06:36:57, tkent wrote:
> Ditto.

I meant 3-line copyright header.

[Dan Beam, 2014-05-07 03:22:58]

FYI: this is stalled as we figure out a behavior that works for everybody
(whatwg@ + public-script-coord@).

[yhirano, 2014-08-19 03:58:46]

Remove myself as a reviewer. Feel free to readd me if/when this comes alive
again.