CallbackPromiseAdapter types should be more compatible with WebCallbacks (3/3).

CallbackPromiseAdapter types should be more compatible with WebCallbacks (3/3).

This series of CLs makes CallbackPromiseAdapter (CPA) more compatible with
WebCallbacks (WC) in terms of type parameters.

Before this CL, CPA<S, T> corresponded to WC<S::WebType*, T::WebType*>. CPAs
took ownership of passed pointers but there was no place to represent that in
WebCallbacks.

This series of CLs changes that: CPA<S, T> correspond to WC<S::WebType,
T::WebType>. CPA users can specify if he/she wants the parameter ownership by
specifying the type parameter. For example, setting S::WebType to OwnPtr<X>
means it takes ownership. Setting S::WebType to |const X&| means it doesn't
take ownership.
WebCallbacks is exposed to chromium side, so we use WebPassOwnPtr as the
counterpart of PassOwnPtr.

[1] https://codereview.chromium.org/1234603003/
[2] https://codereview.chromium.org/1235083006/
[3] This CL.

BUG=493531
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200234

[yhirano, 2015-07-15 06:18:45]

yhirano@chromium.org changed reviewers:
+ kinuko@chromium.org

[kinuko, 2015-07-26 15:15:09]

https://codereview.chromium.org/1240763002/diff/100001/Source/bindings/core/v...
File Source/bindings/core/v8/CallbackPromiseAdapter.h (right):

https://codereview.chromium.org/1240763002/diff/100001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:42: 
nit: for files that heavily use templates I prefer having overview comment at
the beginning of file and have internal templates in a nested namespace like
'internal', so that it becomes clearer what all these templates are about. I'd
probably move the comment at line 163 here and also add a (very) brief
description about how it's implemented.

https://codereview.chromium.org/1240763002/diff/100001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:104: class OnSuccess : public
Base<S, T> {
Um... would it be possible / make sense to define onSuccess / onError behavior
as separate templates and define CallbackPromiseAdapter template as composite of
these?  I feel using composite would make this more straightforward but I might
be wrong.

[yhirano, 2015-07-27 05:30:02]

https://codereview.chromium.org/1240763002/diff/100001/Source/bindings/core/v...
File Source/bindings/core/v8/CallbackPromiseAdapter.h (right):

https://codereview.chromium.org/1240763002/diff/100001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:42: 
On 2015/07/26 15:15:08, kinuko wrote:
> nit: for files that heavily use templates I prefer having overview comment at
> the beginning of file and have internal templates in a nested namespace like
> 'internal', so that it becomes clearer what all these templates are about. I'd
> probably move the comment at line 163 here and also add a (very) brief
> description about how it's implemented.

Done.

CallbackPromiseAdapterInternal in internal is a bit messy, though. It is needed
because blink::internal is shared by many classes.

https://codereview.chromium.org/1240763002/diff/100001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:104: class OnSuccess : public
Base<S, T> {
On 2015/07/26 15:15:08, kinuko wrote:
> Um... would it be possible / make sense to define onSuccess / onError behavior
> as separate templates and define CallbackPromiseAdapter template as composite
of
> these?  I feel using composite would make this more straightforward but I
might
> be wrong.

I wanted to do so but I couldn't. I think the proposed implementation is better
than the existing one that has duplicated onSuccess / onError implementations.

[kinuko, 2015-07-27 05:59:00]

Ok, lgtm

https://codereview.chromium.org/1240763002/diff/100001/Source/bindings/core/v...
File Source/bindings/core/v8/CallbackPromiseAdapter.h (right):

https://codereview.chromium.org/1240763002/diff/100001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:42: 
On 2015/07/27 05:30:02, yhirano wrote:
> On 2015/07/26 15:15:08, kinuko wrote:
> > nit: for files that heavily use templates I prefer having overview comment
at
> > the beginning of file and have internal templates in a nested namespace like
> > 'internal', so that it becomes clearer what all these templates are about.
I'd
> > probably move the comment at line 163 here and also add a (very) brief
> > description about how it's implemented.
> 
> Done.
> 
> CallbackPromiseAdapterInternal in internal is a bit messy, though. It is
needed
> because blink::internal is shared by many classes.

Thanks, I still prefer having them in internal (but let's see what owners say).

https://codereview.chromium.org/1240763002/diff/100001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:104: class OnSuccess : public
Base<S, T> {
On 2015/07/27 05:30:01, yhirano wrote:
> On 2015/07/26 15:15:08, kinuko wrote:
> > Um... would it be possible / make sense to define onSuccess / onError
behavior
> > as separate templates and define CallbackPromiseAdapter template as
composite
> of
> > these?  I feel using composite would make this more straightforward but I
> might
> > be wrong.
> 
> I wanted to do so but I couldn't. I think the proposed implementation is
better
> than the existing one that has duplicated onSuccess / onError implementations.
> 

Hm, ok...

https://codereview.chromium.org/1240763002/diff/120001/Source/bindings/core/v...
File Source/bindings/core/v8/CallbackPromiseAdapter.h (right):

https://codereview.chromium.org/1240763002/diff/120001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:48: //  - If S or T don't have
WebType (e.g. S = bool), the trivial WebType holder
nit: 'the trivial WebType holder' -> 'a default WebType holder called
TrivialWebType holder' ?

(trivial webtype holder is not introduced / defined here yet)

https://codereview.chromium.org/1240763002/diff/120001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:49: //    is used. For example,
CallbackPromiseAdapter<bool, void> is a subclass of
'is a' -> 'can be a' ?

https://codereview.chromium.org/1240763002/diff/120001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:52: //    WebCallbacks is
WebPassOwnPtr<T>, because WebCallbacks is exposed to
'is' -> 'must be' ?

[yhirano, 2015-07-27 06:10:27]

https://codereview.chromium.org/1240763002/diff/120001/Source/bindings/core/v...
File Source/bindings/core/v8/CallbackPromiseAdapter.h (right):

https://codereview.chromium.org/1240763002/diff/120001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:48: //  - If S or T don't have
WebType (e.g. S = bool), the trivial WebType holder
On 2015/07/27 05:59:00, kinuko wrote:
> nit: 'the trivial WebType holder' -> 'a default WebType holder called
> TrivialWebType holder' ?
> 
> (trivial webtype holder is not introduced / defined here yet)

Done.

https://codereview.chromium.org/1240763002/diff/120001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:49: //    is used. For example,
CallbackPromiseAdapter<bool, void> is a subclass of
On 2015/07/27 05:59:00, kinuko wrote:
> 'is a' -> 'can be a' ?

Why? There is no way to have bool and void have their WebTypes, so CPA<bool,
void> is always a subclass of WC<bool, void>.

https://codereview.chromium.org/1240763002/diff/120001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:52: //    WebCallbacks is
WebPassOwnPtr<T>, because WebCallbacks is exposed to
On 2015/07/27 05:59:00, kinuko wrote:
> 'is' -> 'must be' ?

Done.

[kinuko, 2015-07-27 07:43:54]

https://codereview.chromium.org/1240763002/diff/120001/Source/bindings/core/v...
File Source/bindings/core/v8/CallbackPromiseAdapter.h (right):

https://codereview.chromium.org/1240763002/diff/120001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:49: //    is used. For example,
CallbackPromiseAdapter<bool, void> is a subclass of
On 2015/07/27 06:10:27, yhirano wrote:
> On 2015/07/27 05:59:00, kinuko wrote:
> > 'is a' -> 'can be a' ?
> 
> Why? There is no way to have bool and void have their WebTypes, so CPA<bool,
> void> is always a subclass of WC<bool, void>.

Ok, this was impossible before so my mental model was biased (like 'yay, now we
can do this!'). Current wording is fine.

[yhirano, 2015-07-27 12:32:05]

yhirano@chromium.org changed reviewers:
+ haraken@chromium.org, mek@chromium.org, mlamouri@chromium.org,
mvanouwerkerk@chromium.org, peter@chromium.org, scheib@chromium.org

[yhirano, 2015-07-27 12:32:05]

PTAL.

mlamouri@ for modules/presentation/, modules/app_banner,
publc/platform/modules/app_banner, public/platform/modules/presentation.
mvanouwerkerk@ for modules/push_messaging,
public/platform/modules/push_messaging.
scheib@ for public/platform/modules/bluetooth.
mek@ for public/platform/modules/geofencing.
peter@ for modules/notifications, public/platform/modules/notifications.
haraken@ for bindings/

[haraken, 2015-07-27 12:46:39]

bindings/ LGTM

[Marijn Kruisselbrink, 2015-07-27 16:57:24]

geofencing LGTM

[Peter Beverloo, 2015-07-27 17:30:10]

notifications and push lgtm

https://codereview.chromium.org/1240763002/diff/140001/public/platform/module...
File public/platform/modules/push_messaging/WebPushProvider.h (right):

https://codereview.chromium.org/1240763002/diff/140001/public/platform/module...
public/platform/modules/push_messaging/WebPushProvider.h:19: using
WebPushPermissionStatusCallbacks = WebCallbacks<WebPushPermissionStatus*,
WebPushError*>;
Why can't we use const WebPushError& for these two callbacks? That'd allow us to
get rid of a few more *s in push code.

(Happy to take care of this later.)

[scheib, 2015-07-27 17:39:18]

With comment suggestion, bluetooth LGTM

https://codereview.chromium.org/1240763002/diff/140001/Source/bindings/core/v...
File Source/bindings/core/v8/CallbackPromiseAdapter.h (right):

https://codereview.chromium.org/1240763002/diff/140001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:67: //  - trivial WebType
holder (TrivialWebTypeHolder).
Please include sample best-practice use code snippets.

[Michael van Ouwerkerk, 2015-07-28 10:13:51]

push_messaging lgtm

[yhirano, 2015-07-29 04:06:30]

Patchset #10 (id:180001) has been deleted

[yhirano, 2015-07-29 04:08:30]

Patchset #9 (id:160001) has been deleted

[yhirano, 2015-07-29 04:09:47]

yhirano@chromium.org changed reviewers:
+ avayvod@chromium.org
- mlamouri@chromium.org

[yhirano, 2015-07-29 04:21:46]

Patchset #9 (id:200001) has been deleted

[yhirano, 2015-07-29 04:22:35]

I reverted app_banner related changes because it no longer uses
CallbackPromiseAdapter.
avavyvod@, can you take a look at the presentation change?

https://codereview.chromium.org/1240763002/diff/140001/Source/bindings/core/v...
File Source/bindings/core/v8/CallbackPromiseAdapter.h (right):

https://codereview.chromium.org/1240763002/diff/140001/Source/bindings/core/v...
Source/bindings/core/v8/CallbackPromiseAdapter.h:67: //  - trivial WebType
holder (TrivialWebTypeHolder).
On 2015/07/27 17:39:18, scheib wrote:
> Please include sample best-practice use code snippets.

Done.

https://codereview.chromium.org/1240763002/diff/140001/public/platform/module...
File public/platform/modules/push_messaging/WebPushProvider.h (right):

https://codereview.chromium.org/1240763002/diff/140001/public/platform/module...
public/platform/modules/push_messaging/WebPushProvider.h:19: using
WebPushPermissionStatusCallbacks = WebCallbacks<WebPushPermissionStatus*,
WebPushError*>;
On 2015/07/27 17:30:09, Peter Beverloo wrote:
> Why can't we use const WebPushError& for these two callbacks? That'd allow us
to
> get rid of a few more *s in push code.
> 
> (Happy to take care of this later.)

Yeah, that's possible and desirable but changing all WebCallbacks makes this CL
(and other CLs in this series) much bigger. After this CL is landed, it will be
possible to fix callbacks module by module, so I'd like to fix them later.

[yhirano, 2015-08-04 03:36:53]

ping

[whywhat, 2015-08-05 12:48:29]

On 2015/08/04 at 03:36:53, yhirano wrote:
> ping

presentation/ lgtm

[yhirano, 2015-08-06 01:12:21]

yhirano@chromium.org changed reviewers:
+ tkent@chromium.org

[yhirano, 2015-08-06 01:12:22]

tkent@, can you take a look at public/ changes?

[tkent, 2015-08-06 02:13:56]

public lgtm

[yhirano, 2015-08-10 04:33:54]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2015-08-10 04:33:55]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
peter@chromium.org, scheib@chromium.org, mvanouwerkerk@chromium.org,
haraken@chromium.org, mek@chromium.org
Link to the patchset: https://codereview.chromium.org/1240763002/#ps260001
(title: "rebase")

[commit-bot: I haz the power, 2015-08-10 04:33:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1240763002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1240763002/260001

[commit-bot: I haz the power, 2015-08-10 05:55:37]

Committed patchset #11 (id:260001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200234

[Sébastien Marchand, 2015-08-11 20:51:51]

sebmarchand@chromium.org changed reviewers:
+ sebmarchand@chromium.org

[Sébastien Marchand, 2015-08-11 20:51:51]

This is breaking the VS2015 builder: 

FAILED: ninja -t msvc -e environment.x86 --
"C:\b\depot_tools\win_toolchain\vs_files\VC\bin\amd64_x86\cl.exe" /nologo
/showIncludes /FC
@obj\third_party\WebKit\Source\modules\bluetooth\modules.BluetoothGATTCharacteristic.obj.rsp
/c
..\..\third_party\WebKit\Source\modules\bluetooth\BluetoothGATTCharacteristic.cpp
/Foobj\third_party\WebKit\Source\modules\bluetooth\modules.BluetoothGATTCharacteristic.obj
/Fdobj\third_party\WebKit\Source\modules\modules.cc.pdb 
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\bindings\core\v8\callbackpromiseadapter.h(159):
error C2182: 'r': illegal use of type 'void'
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\bindings\core\v8\callbackpromiseadapter.h(181):
note: see reference to class template instantiation
'blink::internal::CallbackPromiseAdapterInternal::OnSuccess<S,T>' being compiled
        with
        [
            S=blink::internal::CallbackPromiseAdapterTrivialWebTypeHolder<void>,
            T=blink::BluetoothError
        ]
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\bindings\core\v8\callbackpromiseadapter.h(208):
note: see reference to class template instantiation
'blink::internal::CallbackPromiseAdapterInternal::OnError<blink::internal::CallbackPromiseAdapterTrivialWebTypeHolder<void>,blink::BluetoothError>'
being compiled
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\modules\bluetooth\bluetoothgattcharacteristic.cpp(61):
note: see reference to class template instantiation
'blink::internal::CallbackPromiseAdapterInternal::CallbackPromiseAdapter<void,blink::BluetoothError>'
being compiled
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\bindings\core\v8\callbackpromiseadapter.h(159):
error C3668:
'blink::internal::CallbackPromiseAdapterInternal::OnSuccess<S,T>::onSuccess':
method with override specifier 'override' did not override any base class
methods
        with
        [
            S=blink::internal::CallbackPromiseAdapterTrivialWebTypeHolder<void>,
            T=blink::BluetoothError
        ]
FAILED: ninja -t msvc -e environment.x86 --
"C:\b\depot_tools\win_toolchain\vs_files\VC\bin\amd64_x86\cl.exe" /nologo
/showIncludes /FC
@obj\third_party\WebKit\Source\modules\cachestorage\modules.Cache.obj.rsp /c
..\..\third_party\WebKit\Source\modules\cachestorage\Cache.cpp
/Foobj\third_party\WebKit\Source\modules\cachestorage\modules.Cache.obj
/Fdobj\third_party\WebKit\Source\modules\modules.cc.pdb 
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\bindings\core\v8\callbackpromiseadapter.h(159):
error C2182: 'r': illegal use of type 'void'
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\bindings\core\v8\callbackpromiseadapter.h(181):
note: see reference to class template instantiation
'blink::internal::CallbackPromiseAdapterInternal::OnSuccess<S,T>' being compiled
        with
        [
            S=blink::internal::CallbackPromiseAdapterTrivialWebTypeHolder<void>,
            T=blink::CacheStorageError
        ]
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\bindings\core\v8\callbackpromiseadapter.h(208):
note: see reference to class template instantiation
'blink::internal::CallbackPromiseAdapterInternal::OnError<blink::internal::CallbackPromiseAdapterTrivialWebTypeHolder<void>,blink::CacheStorageError>'
being compiled
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\modules\cachestorage\cache.cpp(198):
note: see reference to class template instantiation
'blink::internal::CallbackPromiseAdapterInternal::CallbackPromiseAdapter<void,blink::CacheStorageError>'
being compiled
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\bindings\core\v8\callbackpromiseadapter.h(159):
error C3668:
'blink::internal::CallbackPromiseAdapterInternal::OnSuccess<S,T>::onSuccess':
method with override specifier 'override' did not override any base class
methods
        with
        [
            S=blink::internal::CallbackPromiseAdapterTrivialWebTypeHolder<void>,
            T=blink::CacheStorageError
        ]
ninja: build stopped: subcommand failed.



Is it possible to fix this or to revert this change until we figure out how to
fix it ? (I've tried to look at this, but there's so many templates... my eyes
are bleeding).

[Sébastien Marchand, 2015-08-11 20:52:24]

(here's the URL of the builder:
http://build.chromium.org/p/chromium.fyi/builders/Chromium%20Builder)

[brucedawson, 2015-08-12 00:41:30]

I tried poking at this to figure out what's going on but I'm not having much
luck. Here's what I've found.

The trail starts at the "new CallbackPromiseAdapter<void,
BluetoothError>(resolver)" code at the end of bluetoothgattcharacteristic.cpp.

That goes to this code:
    template <typename S, typename T>
    class CallbackPromiseAdapter final : public OnError<WebTypeHolder<S>,
WebTypeHolder<T>> {
        WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter);
    public:
        explicit CallbackPromiseAdapter(ScriptPromiseResolver* resolver) :
OnError<WebTypeHolder<S>, WebTypeHolder<T>>(resolver) {}
    };

with the types of S and T being
blink::internal::CallbackPromiseAdapterTrivialWebTypeHolder<void> and
blink::BluetoothError. So far so good, I think.

Then it goes to the two template argument version of OnError with S and T still
the same. That's reasonable.

Then it ends up in the two argument version of OnSuccess where it ends up
declaring an onSuccess function that takes 'void'. That's no good.

My assumption is that it is supposed to end up in the one argument version of
OnSuccess. I think it doesn't get there because it doesn't match
CallbackPromiseAdapterTrivialWebTypeHolder<void> to WebTypeHolder<void>.

At this point I've made enough assumptions that I don't trust my analysis very
much. Can somebody sanity check what is *supposed* to happen and then I can
compare against what is actually happen and make a repro. Any suggestions on a
workaround would also be appreciated.

[yhirano, 2015-08-12 02:01:25]

On 2015/08/12 00:41:30, brucedawson wrote:
> I tried poking at this to figure out what's going on but I'm not having much
> luck. Here's what I've found.
> 
> The trail starts at the "new CallbackPromiseAdapter<void,
> BluetoothError>(resolver)" code at the end of bluetoothgattcharacteristic.cpp.
> 
> That goes to this code:
>     template <typename S, typename T>
>     class CallbackPromiseAdapter final : public OnError<WebTypeHolder<S>,
> WebTypeHolder<T>> {
>         WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter);
>     public:
>         explicit CallbackPromiseAdapter(ScriptPromiseResolver* resolver) :
> OnError<WebTypeHolder<S>, WebTypeHolder<T>>(resolver) {}
>     };
> 
> with the types of S and T being
> blink::internal::CallbackPromiseAdapterTrivialWebTypeHolder<void> and
> blink::BluetoothError. So far so good, I think.
> 
> Then it goes to the two template argument version of OnError with S and T
still
> the same. That's reasonable.
> 
> Then it ends up in the two argument version of OnSuccess where it ends up
> declaring an onSuccess function that takes 'void'. That's no good.
> 
> My assumption is that it is supposed to end up in the one argument version of
> OnSuccess. I think it doesn't get there because it doesn't match
> CallbackPromiseAdapterTrivialWebTypeHolder<void> to WebTypeHolder<void>.
> 
> At this point I've made enough assumptions that I don't trust my analysis very
> much. Can somebody sanity check what is *supposed* to happen and then I can
> compare against what is actually happen and make a repro. Any suggestions on a
> workaround would also be appreciated.

I would like to fix it with your help because I don't have a Windows
workstation.

> My assumption is that it is supposed to end up in the one argument version of
> OnSuccess. I think it doesn't get there because it doesn't match
> CallbackPromiseAdapterTrivialWebTypeHolder<void> to WebTypeHolder<void>.

Yes. These two are equivalent (see L116-L118). If using the former at L169
solves
the problem, I'm fine with it.

[Sébastien Marchand, 2015-08-12 02:12:21]

If you want I can give you access to a Windows VM with everything already
set up (a checkout of Chrome, the vs2015 toolchain in depot_tools and
Sublime Text)
Le 2015-08-11 10:01 PM, <yhirano@chromium.org> a écrit :

> On 2015/08/12 00:41:30, brucedawson wrote:
>
>> I tried poking at this to figure out what's going on but I'm not having
>> much
>> luck. Here's what I've found.
>>
>
> The trail starts at the "new CallbackPromiseAdapter<void,
>> BluetoothError>(resolver)" code at the end of
>> bluetoothgattcharacteristic.cpp.
>>
>
> That goes to this code:
>>      template <typename S, typename T>
>>      class CallbackPromiseAdapter final : public OnError<WebTypeHolder<S>,
>> WebTypeHolder<T>> {
>>          WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter);
>>      public:
>>          explicit CallbackPromiseAdapter(ScriptPromiseResolver* resolver)
>> :
>> OnError<WebTypeHolder<S>, WebTypeHolder<T>>(resolver) {}
>>      };
>>
>
> with the types of S and T being
>> blink::internal::CallbackPromiseAdapterTrivialWebTypeHolder<void> and
>> blink::BluetoothError. So far so good, I think.
>>
>
> Then it goes to the two template argument version of OnError with S and T
>>
> still
>
>> the same. That's reasonable.
>>
>
> Then it ends up in the two argument version of OnSuccess where it ends up
>> declaring an onSuccess function that takes 'void'. That's no good.
>>
>
> My assumption is that it is supposed to end up in the one argument version
>> of
>> OnSuccess. I think it doesn't get there because it doesn't match
>> CallbackPromiseAdapterTrivialWebTypeHolder<void> to WebTypeHolder<void>.
>>
>
> At this point I've made enough assumptions that I don't trust my analysis
>> very
>> much. Can somebody sanity check what is *supposed* to happen and then I
>> can
>> compare against what is actually happen and make a repro. Any suggestions
>> on a
>> workaround would also be appreciated.
>>
>
> I would like to fix it with your help because I don't have a Windows
> workstation.
>
> My assumption is that it is supposed to end up in the one argument version
>> of
>> OnSuccess. I think it doesn't get there because it doesn't match
>> CallbackPromiseAdapterTrivialWebTypeHolder<void> to WebTypeHolder<void>.
>>
>
> Yes. These two are equivalent (see L116-L118). If using the former at L169
> solves
> the problem, I'm fine with it.
>
>
> https://codereview.chromium.org/1240763002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[yhirano, 2015-08-12 02:20:42]

On 2015/08/12 02:12:21, Sébastien Marchand wrote:
> If you want I can give you access to a Windows VM with everything already
> set up (a checkout of Chrome, the vs2015 toolchain in depot_tools and
> Sublime Text)
> Le 2015-08-11 10:01 PM, <mailto:yhirano@chromium.org> a écrit :
> 
Thanks, that would be of much help.

[Sébastien Marchand, 2015-08-12 14:07:21]

I just sent you the instructions. (FYI, the machine is hosted on Compute
Engine)

On Tue, Aug 11, 2015 at 10:20 PM, <yhirano@chromium.org> wrote:

> On 2015/08/12 02:12:21, Sébastien Marchand wrote:
>
>> If you want I can give you access to a Windows VM with everything already
>> set up (a checkout of Chrome, the vs2015 toolchain in depot_tools and
>> Sublime Text)
>> Le 2015-08-11 10:01 PM, <mailto:yhirano@chromium.org> a écrit :
>>
>
> Thanks, that would be of much help.
>
>
> https://codereview.chromium.org/1240763002/
>



-- 
Sébastien Marchand | Software Developer | sebmarchand@google.com

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[brucedawson, 2015-08-12 22:53:03]

I created preprocessed reproes of bluetoothgattcharacteristic.cpp (one each for
VS 2013 and VS 2015) which I used to file a bug with Microsoft.

If it turns out not to be a VS bug we can always close it.

https://connect.microsoft.com/VisualStudio/feedback/details/1660945