[Presentation API] PresentationRequest::getAvailability() should return the same promise

[Presentation API] PresentationRequest::getAvailability() should return the same promise

Add a ScriptPromiseProperty to PresentationRequest to store PresentationAvailability promise object. getAvailability() will always return the same promise object.

BUG=653131
Committed: https://crrev.com/9c627c11ac9fbcd803153a5bd77839cabaf29c96
Cr-Commit-Position: refs/heads/master@{#439269}

[zhaobin, 2016-12-12 22:31:16]

zhaobin@chromium.org changed reviewers:
+ mfoltz@chromium.org, mlamouri@chromium.org

[mlamouri (slow - plz ping), 2016-12-13 13:56:12]

Looks great! Can you add a LayoutTest that checks that the returned promise is
the same object (ie. using `===`).

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/presentation/PresentationAvailability.h
(right):

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/presentation/PresentationAvailability.h:36:
static PresentationAvailability* take(PresentationAvailabilityProperty*,
Can you forward declare one way or another?

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/presentation/PresentationError.cpp
(right):

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/presentation/PresentationError.cpp:14:
DOMException* PresentationError::take(const WebPresentationError& error) {
For next time, in the spirit of keeping the CLs atomic, you could have done this
change in a follow-up :)

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/presentation/PresentationRequest.h
(right):

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/presentation/PresentationRequest.h:31: class
MODULES_EXPORT PresentationRequest final
Why did this become MODULES_EXPORT?

[zhaobin, 2016-12-14 02:25:09]

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/presentation/PresentationAvailability.h
(right):

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/presentation/PresentationAvailability.h:36:
static PresentationAvailability* take(PresentationAvailabilityProperty*,
On 2016/12/13 13:56:12, mlamouri wrote:
> Can you forward declare one way or another?

Cannot forward declare using. Create a seperate .h file for this...

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/presentation/PresentationError.cpp
(right):

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/presentation/PresentationError.cpp:14:
DOMException* PresentationError::take(const WebPresentationError& error) {
On 2016/12/13 13:56:12, mlamouri wrote:
> For next time, in the spirit of keeping the CLs atomic, you could have done
this
> change in a follow-up :)

Sure :)

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/presentation/PresentationRequest.h
(right):

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/presentation/PresentationRequest.h:31: class
MODULES_EXPORT PresentationRequest final
On 2016/12/13 13:56:12, mlamouri wrote:
> Why did this become MODULES_EXPORT?

Needed in unit tests.

[mlamouri (slow - plz ping), 2016-12-14 12:25:06]

lgtm

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/presentation/PresentationAvailability.h
(right):

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/presentation/PresentationAvailability.h:36:
static PresentationAvailability* take(PresentationAvailabilityProperty*,
On 2016/12/14 at 02:25:09, zhaobin wrote:
> On 2016/12/13 13:56:12, mlamouri wrote:
> > Can you forward declare one way or another?
> 
> Cannot forward declare using. Create a seperate .h file for this...

An alternative would be to simply re-declare the template name in this header,
wouldn't it work?

[zhaobin, 2016-12-14 20:51:53]

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/presentation/PresentationAvailability.h
(right):

https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/presentation/PresentationAvailability.h:36:
static PresentationAvailability* take(PresentationAvailabilityProperty*,
On 2016/12/14 12:25:06, mlamouri wrote:
> On 2016/12/14 at 02:25:09, zhaobin wrote:
> > On 2016/12/13 13:56:12, mlamouri wrote:
> > > Can you forward declare one way or another?
> > 
> > Cannot forward declare using. Create a seperate .h file for this...
> 
> An alternative would be to simply re-declare the template name in this header,
> wouldn't it work?

Yes, it works. It concerns me a little because we need to declear the same using
in PresentationRequest.h, PresentationAvailability.h and
PresentationAvailabilityCallbacks.h. Would creating a .h file better in this
case?

[mlamouri (slow - plz ping), 2016-12-15 10:07:26]

On 2016/12/14 at 20:51:53, zhaobin wrote:
>
https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
> File third_party/WebKit/Source/modules/presentation/PresentationAvailability.h
(right):
> 
>
https://codereview.chromium.org/2572473003/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/presentation/PresentationAvailability.h:36:
static PresentationAvailability* take(PresentationAvailabilityProperty*,
> On 2016/12/14 12:25:06, mlamouri wrote:
> > On 2016/12/14 at 02:25:09, zhaobin wrote:
> > > On 2016/12/13 13:56:12, mlamouri wrote:
> > > > Can you forward declare one way or another?
> > > 
> > > Cannot forward declare using. Create a seperate .h file for this...
> > 
> > An alternative would be to simply re-declare the template name in this
header,
> > wouldn't it work?
> 
> Yes, it works. It concerns me a little because we need to declear the same
using in PresentationRequest.h, PresentationAvailability.h and
PresentationAvailabilityCallbacks.h. Would creating a .h file better in this
case?

Didn't realise you needed this in three places. The header sounds better then :)

[mark a. foltz, 2016-12-15 22:23:36]

lgtm

[zhaobin, 2016-12-15 22:27:52]

The CQ bit was checked by zhaobin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-15 22:28:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-15 23:19:53]

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

[commit-bot: I haz the power, 2016-12-15 23:19:54]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[zhaobin, 2016-12-15 23:41:33]

The CQ bit was checked by zhaobin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-15 23:42:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-16 02:16:09]

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

[commit-bot: I haz the power, 2016-12-16 02:16:10]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[zhaobin, 2016-12-16 19:11:07]

The CQ bit was checked by zhaobin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-16 19:11:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-17 00:18:55]

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

[commit-bot: I haz the power, 2016-12-17 00:18:57]

Dry run: This issue passed the CQ dry run.

[zhaobin, 2016-12-17 00:57:35]

The CQ bit was checked by zhaobin@chromium.org

[zhaobin, 2016-12-17 00:57:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlamouri@chromium.org, mfoltz@chromium.org
Link to the patchset: https://codereview.chromium.org/2572473003/#ps80001
(title: "rebase with master")

[commit-bot: I haz the power, 2016-12-17 00:58:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-17 01:05:04]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481936255562340,
"parent_rev": "c221ec91819a2dac6a32aaebcb10aaff45f41a5b", "commit_rev":
"ba6ef0af743f7405a97a2c9f5c452be6491331b8"}

[commit-bot: I haz the power, 2016-12-17 01:05:30]

Description was changed from

==========
[Presentation API] PresentationRequest::getAvailability() should return the same
promise

Add a ScriptPromiseProperty to PresentationRequest to store
PresentationAvailability promise object. getAvailability() will always return
the same promise object.

BUG=653131
==========

to

==========
[Presentation API] PresentationRequest::getAvailability() should return the same
promise

Add a ScriptPromiseProperty to PresentationRequest to store
PresentationAvailability promise object. getAvailability() will always return
the same promise object.

BUG=653131

Review-Url: https://codereview.chromium.org/2572473003
==========

[commit-bot: I haz the power, 2016-12-17 01:05:31]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-12-17 01:08:48]

Description was changed from

==========
[Presentation API] PresentationRequest::getAvailability() should return the same
promise

Add a ScriptPromiseProperty to PresentationRequest to store
PresentationAvailability promise object. getAvailability() will always return
the same promise object.

BUG=653131

Review-Url: https://codereview.chromium.org/2572473003
==========

to

==========
[Presentation API] PresentationRequest::getAvailability() should return the same
promise

Add a ScriptPromiseProperty to PresentationRequest to store
PresentationAvailability promise object. getAvailability() will always return
the same promise object.

BUG=653131

Committed: https://crrev.com/9c627c11ac9fbcd803153a5bd77839cabaf29c96
Cr-Commit-Position: refs/heads/master@{#439269}
==========

[commit-bot: I haz the power, 2016-12-17 01:08:49]

Patchset 5 (id:??) landed as
https://crrev.com/9c627c11ac9fbcd803153a5bd77839cabaf29c96
Cr-Commit-Position: refs/heads/master@{#439269}