Closed Bug 1466221 Opened 6 years ago Closed 6 years ago

Add ToJSValue specialisation for objects that do not inherit from nsWrapperCache but implement WrapObject

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla62

Tracking Flags:

Tracking

Status

firefox62

---

fixed

People

(Reporter: jya, Assigned: bzbarsky)

References

Details

Attachments

(3 files, 6 obsolete files)

0001-Bug-1409664-P1.-Add-MediaCapabilities-skeleton-IDL.-.patch 6 years ago Jean-Yves Avenard [:jya] 17.97 KB, patch		Details \| Diff \| Splinter Review
0002-Bug-1409664-P2.-Add-ScreenLuminance-objects-and-frie.patch 6 years ago Jean-Yves Avenard [:jya] 6.24 KB, patch		Details \| Diff \| Splinter Review
0003-amend-p1.patch 6 years ago Jean-Yves Avenard [:jya] 3.32 KB, patch		Details \| Diff \| Splinter Review
0004-amend-p1-with-wrapperCache.patch 6 years ago Jean-Yves Avenard [:jya] 5.70 KB, patch		Details \| Diff \| Splinter Review
0005-p3-resolve-MediaCapabilities.patch 6 years ago Jean-Yves Avenard [:jya] 5.70 KB, patch		Details \| Diff \| Splinter Review
Implement ToJSValue variants for non-refcounted (so owned) non-wrapper-cached DOM objects 6 years ago Boris Zbarsky [:bzbarsky] 6.22 KB, patch		Details \| Diff \| Splinter Review
Implement ToJSValue variants for non-refcounted (so owned) non-wrapper-cached DOM objects 6 years ago Boris Zbarsky [:bzbarsky] 6.22 KB, patch	peterv : review+	Details \| Diff \| Splinter Review
. P2. Use forwarding reference in MaybeResolve. r?bz 6 years ago Jean-Yves Avenard [:jya] 3.29 KB, patch	bzbarsky : review+	Details \| Diff \| Splinter Review
Something like this and then kill off the conditions on ToJSValue? 6 years ago Boris Zbarsky [:bzbarsky] 1.92 KB, patch	peterv : review+	Details \| Diff \| Splinter Review

Jean-Yves Avenard [:jya]

Reporter

Description

•

6 years ago

Currently, you can't resolve a Promise with a class that do not inherit from nsWrapperCache due to missing ToJSValue template specialization.

For classes providing WrapObject method, we should use that.

Jean-Yves Avenard [:jya]

Reporter

Comment 1

•

6 years ago

Attached patch 0001-Bug-1409664-P1.-Add-MediaCapabilities-skeleton-IDL.-.patch (obsolete) — Details — Splinter Review

Jean-Yves Avenard [:jya]

Reporter

Comment 2

•

6 years ago

Attached patch 0002-Bug-1409664-P2.-Add-ScreenLuminance-objects-and-frie.patch (obsolete) — Details — Splinter Review

Jean-Yves Avenard [:jya]

Reporter

Comment 3

•

6 years ago

Attached patch 0003-amend-p1.patch (obsolete) — Details — Splinter Review

Jean-Yves Avenard [:jya]

Reporter

Comment 4

•

6 years ago

Attached patch 0004-amend-p1-with-wrapperCache.patch (obsolete) — Details — Splinter Review

Jean-Yves Avenard [:jya]

Reporter

Comment 5

•

6 years ago

Attached patch 0005-p3-resolve-MediaCapabilities.patch (obsolete) — Details — Splinter Review

Boris Zbarsky [:bzbarsky]

Assignee

Comment 6

•

6 years ago

I think it's pretty reasonable to do this for non-refcounted things, where we can guarantee that we hand ownership to the JSObject and no one can try to convert them to JS _again_.

For refcounted things, I am not sure I want to enable this; it would be too easy to misuse.

Boris Zbarsky [:bzbarsky]

Assignee

Comment 7

•

6 years ago

Attached patch Implement ToJSValue variants for non-refcounted (so owned) non-wrapper-cached DOM objects (obsolete) — Details — Splinter Review

Attachment #8982661 - Flags: review?(peterv)

Boris Zbarsky [:bzbarsky]

Assignee

Updated

•

6 years ago

Assignee: nobody → bzbarsky

Status: NEW → ASSIGNED

Jean-Yves Avenard [:jya]

Reporter

Comment 8

•

6 years ago

Comment on attachment 8982661 [details] [diff] [review]
Implement ToJSValue variants for non-refcounted (so owned) non-wrapper-cached DOM objects

Review of attachment 8982661 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/ToJSValue.h
@@ +172,5 @@
> +  MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
> +
> +  // This is a cut-down version of
> +  // WrapNewBindingNonWrapperCachedObject that doesn't need to deal
> +  // with nearly as may cases.

nit: as many cases

Boris Zbarsky [:bzbarsky]

Assignee

Comment 9

•

6 years ago

Attached patch Implement ToJSValue variants for non-refcounted (so owned) non-wrapper-cached DOM objects — Details — Splinter Review

Good catch!

Attachment #8982769 - Flags: review?(peterv)

Boris Zbarsky [:bzbarsky]

Assignee

Updated

•

6 years ago

Attachment #8982661 - Attachment is obsolete: true

Attachment #8982661 - Flags: review?(peterv)

Jean-Yves Avenard [:jya]

Reporter

Comment 10

•

6 years ago

With the following class definition:
class MediaCapabilitiesInfo final : public NonRefcountedDOMObject
{
public:

  // WebIDL methods
  bool Supported() const
  {
    return mSupported;
  }
  bool Smooth() const
  {
    return mSmooth;
  }
  bool PowerEfficient() const
  {
    return mPowerEfficient;
  }
  // End WebIDL methods

  MediaCapabilitiesInfo(bool aSupported, bool aSmooth, bool aPowerEfficient)
    : mSupported(aSupported)
    , mSmooth(aSmooth)
    , mPowerEfficient(aPowerEfficient)
  {
  }

  bool WrapObject(JSContext* aCx,
                  JS::Handle<JSObject*> aGivenProto,
                  JS::MutableHandle<JSObject*> aReflector);

private:
  bool mSupported;
  bool mSmooth;
  bool mPowerEfficient;
};

I get:
 0:10.25 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dom/bindings/UnifiedBindings10.cpp:146:
 0:10.25 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dom/bindings/MediaCapabilitiesBinding.cpp:1591:21: error: no member named 'GetWrapperPreserveColor' in 'mozilla::dom::MediaCapabilitiesInfo'
 0:10.25   if (self && self->GetWrapperPreserveColor()) {
 0:10.25               ~~~~  ^
 0:10.25 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dom/bindings/MediaCapabilitiesBinding.cpp:1762:72: error: no member named 'GetParentObject' in 'mozilla::dom::MediaCapabilitiesInfo'
 0:10.25   JS::Rooted<JSObject*> global(aCx, FindAssociatedGlobal(aCx, aObject->GetParentObject()));
 0:10.25                                                               ~~~~~~~  ^
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dom/bindings/UnifiedBindings10.cpp:2:
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dom/bindings/MIDIAccessBinding.cpp:3:
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/EventHandlerBinding.h:14:
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:13:
 0:11.26 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/BindingUtils.h:2657:14: error: no member named 'PreserveWrapper' in 'mozilla::dom::MediaCapabilitiesInfo'
 0:11.26     aObject->PreserveWrapper(aObject, NS_CYCLE_COLLECTION_PARTICIPANT(T));
 0:11.26     ~~~~~~~  ^
 0:11.26 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/BindingUtils.h:2673:29: note: in instantiation of member function 'mozilla::dom::PreserveWrapperHelper<mozilla::dom::MediaCapabilitiesInfo, false>::PreserveWrapper' requested here
 0:11.26   PreserveWrapperHelper<T>::PreserveWrapper(aObject);
 0:11.26                             ^
 0:11.26 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dom/bindings/MediaCapabilitiesBinding.cpp:1592:5: note: in instantiation of function template specialization 'mozilla::dom::PreserveWrapper<mozilla::dom::MediaCapabilitiesInfo>' requested here
 0:11.26     PreserveWrapper(self);
 0:11.26     ^
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dom/bindings/UnifiedBindings10.cpp:2:
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dom/bindings/MIDIAccessBinding.cpp:3:
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/EventHandlerBinding.h:6:
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/GeckoProfiler.h:73:
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/nsIURI.h:10:
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/nsISupports.h:77:
 0:11.26 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/nsISupportsUtils.h:134:19: error: no member named 'QueryInterface' in 'mozilla::dom::MediaCapabilitiesInfo'
 0:11.26   return aSource->QueryInterface(NS_GET_TEMPLATE_IID(DestinationType),
 0:11.26          ~~~~~~~  ^
 0:11.26 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/BindingUtils.h:1425:3: note: in instantiation of function template specialization 'CallQueryInterface<mozilla::dom::MediaCapabilitiesInfo, nsWrapperCache>' requested here
 0:11.26   CallQueryInterface(p, &cache);
 0:11.26   ^
 0:11.26 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dom/bindings/MediaCapabilitiesBinding.cpp:1602:5: note: in instantiation of function template specialization 'mozilla::dom::ClearWrapper<mozilla::dom::MediaCapabilitiesInfo>' requested here
 0:11.26     ClearWrapper(self, self, obj);
 0:11.26     ^
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dom/bindings/UnifiedBindings10.cpp:2:
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dom/bindings/MIDIAccessBinding.cpp:3:
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/EventHandlerBinding.h:14:
 0:11.26 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:13:
 0:11.26 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/BindingUtils.h:1443:3: error: no matching function for call to 'CallQueryInterface'
 0:11.26   CallQueryInterface(p, &cache);
 0:11.26   ^~~~~~~~~~~~~~~~~~
 0:11.26 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dom/bindings/MediaCapabilitiesBinding.cpp:1612:5: note: in instantiation of function template specialization 'mozilla::dom::UpdateWrapper<mozilla::dom::MediaCapabilitiesInfo>' requested here
 0:11.26     UpdateWrapper(self, self, obj, old);
 0:11.26     ^
 0:11.26 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/nsISupportsUtils.h:140:1: note: candidate template ignored: could not match 'RefPtr<type-parameter-0-0>' against 'mozilla::dom::MediaCapabilitiesInfo *'
 0:11.26 CallQueryInterface(RefPtr<SourceType>& aSourcePtr, DestinationType** aDestPtr)
 0:11.26 ^
 0:11.26 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/nsCOMPtr.h:1381:1: note: candidate template ignored: could not match 'nsGetterAddRefs<type-parameter-0-1>' against 'nsWrapperCache **'
 0:11.26 CallQueryInterface(T* aSource, nsGetterAddRefs<DestinationType> aDestination)
 0:11.26 ^
 0:11.27 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/nsCOMPtr.h:1510:1: note: candidate template ignored: could not match 'nsCOMPtr<type-parameter-0-0>' against 'mozilla::dom::MediaCapabilitiesInfo *'
 0:11.27 CallQueryInterface(nsCOMPtr<SourceType>& aSourcePtr, DestinationType** aDestPtr)
 0:11.27 ^
 0:11.27 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/nsISupportsUtils.h:123:1: note: candidate template ignored: substitution failure [with T = mozilla::dom::MediaCapabilitiesInfo, DestinationType = nsWrapperCache]
 0:11.27 CallQueryInterface(T* aSource, DestinationType** aDestination)
 0:11.27 ^
 0:11.43 5 errors generated.
 0:11.46 make[4]: *** [UnifiedBindings10.o] Error 1
 0:11.46 make[3]: *** [dom/bindings/target] Error 2
 0:17.66 make[2]: *** [compile] Error 2
 0:17.67 make[1]: *** [default] Error 2
 0:17.67 make: *** [build] Error 2

So we need a GetWrapperPreserve* for all webidl accessor.
We must define a GetParentObject which can of defeat the purpose if we need to take care of cycle collection later.

error: no member named 'PreserveWrapper' in 'mozilla::dom::MediaCapabilitiesInfo'
 0:11.26     aObject->PreserveWrapper(aObject, NS_CYCLE_COLLECTION_PARTICIPANT(T));

Not sure we're heading on getting something simpler for very simplistic class container

Jean-Yves Avenard [:jya]

Reporter

Comment 11

•

6 years ago

oh my bad, I forgot to re-add:
'MediaCapabilitiesInfo' : {
    'wrapperCache': False,
},

into Bindings.conf.

With that fixed and doing:
  auto info = MakeUnique<MediaCapabilitiesInfo>(true, true, true);
  promise->MaybeResolve(std::move(info));


 0:06.90 In file included from /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dom/media/mediacapabilities/Unified_cpp_mediacapabilities0.cpp:2:
 0:06.91 In file included from /Users/jyavenard/Work/Mozilla/mozilla-central/dom/media/mediacapabilities/MediaCapabilities.cpp:12:
 0:06.91 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/Promise.h:192:10: error: no matching function for call to 'ToJSValue'
 0:06.91     if (!ToJSValue(cx, aArgument, &val)) {
 0:06.91          ^~~~~~~~~
 0:06.91 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/Promise.h:76:5: note: in instantiation of function template specialization 'mozilla::dom::Promise::MaybeSomething<const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> > >' requested here
 0:06.91     MaybeSomething(aArg, &Promise::MaybeResolve);
 0:06.91     ^
 0:06.91 /Users/jyavenard/Work/Mozilla/mozilla-central/dom/media/mediacapabilities/MediaCapabilities.cpp:58:12: note: in instantiation of function template specialization 'mozilla::dom::Promise::MaybeResolve<mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> > >' requested here
 0:06.91   promise->MaybeResolve(std::move(info));
 0:06.91            ^
 0:06.91 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ClientsBinding.h:43:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'mozilla::dom::ClientType' for 2nd argument
 0:06.91 ToJSValue(JSContext* aCx, ClientType aArgument, JS::MutableHandle<JS::Value> aValue);
 0:06.91 ^
 0:06.91 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/DocumentBinding.h:44:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'mozilla::dom::VisibilityState' for 2nd argument
 0:06.91 ToJSValue(JSContext* aCx, VisibilityState aArgument, JS::MutableHandle<JS::Value> aValue);
 0:06.91 ^
 0:06.91 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/DocumentBinding.h:60:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'mozilla::dom::FlashClassification' for 2nd argument
 0:06.91 ToJSValue(JSContext* aCx, FlashClassification aArgument, JS::MutableHandle<JS::Value> aValue);
 0:06.91 ^
 0:06.91 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ClientBinding.h:43:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'mozilla::dom::FrameType' for 2nd argument
 0:06.91 ToJSValue(JSContext* aCx, FrameType aArgument, JS::MutableHandle<JS::Value> aValue);
 0:06.91 ^
 0:06.91 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ChromeUtilsBinding.h:53:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'mozilla::dom::Base64URLDecodePadding' for 2nd argument
 0:06.91 ToJSValue(JSContext* aCx, Base64URLDecodePadding aArgument, JS::MutableHandle<JS::Value> aValue);
 0:06.91 ^
 0:06.91 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/CSSStyleSheetBinding.h:43:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'mozilla::dom::CSSStyleSheetParsingMode' for 2nd argument
 0:06.92 ToJSValue(JSContext* aCx, CSSStyleSheetParsingMode aArgument, JS::MutableHandle<JS::Value> aValue);
 0:06.92 ^
 0:06.92 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/MediaRecorderBinding.h:41:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'mozilla::dom::RecordingState' for 2nd argument
 0:06.92 ToJSValue(JSContext* aCx, RecordingState aArgument, JS::MutableHandle<JS::Value> aValue);
 0:06.92 ^
 0:06.92 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:32:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'const nsAString' (aka 'const nsTSubstring<char16_t>') for 2nd argument
 0:06.92 ToJSValue(JSContext* aCx,
 0:06.92 ^
 0:06.92 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:57:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'int32_t' (aka 'int') for 2nd argument
 0:06.92 ToJSValue(JSContext* aCx,
 0:06.92 ^
 0:06.92 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:69:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'uint32_t' (aka 'unsigned int') for 2nd argument
 0:06.92 ToJSValue(JSContext* aCx,
 0:06.92 ^
 0:06.92 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:81:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'int64_t' (aka 'long long') for 2nd argument
 0:06.92 ToJSValue(JSContext* aCx,
 0:06.92 ^
 0:06.92 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:93:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'uint64_t' (aka 'unsigned long long') for 2nd argument
 0:06.92 ToJSValue(JSContext* aCx,
 0:06.92 ^
 0:06.92 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:106:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'float' for 2nd argument
 0:06.92 ToJSValue(JSContext* aCx,
 0:06.92 ^
 0:06.92 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:118:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'double' for 2nd argument
 0:06.92 ToJSValue(JSContext* aCx,
 0:06.93 ^
 0:06.93 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:131:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'mozilla::dom::CallbackObject &' for 2nd argument
 0:06.93 ToJSValue(JSContext* aCx,
 0:06.93 ^
 0:06.93 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:311:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'const JS::Value' for 2nd argument
 0:06.93 ToJSValue(JSContext* aCx, const JS::Value& aArgument,
 0:06.93 ^
 0:06.93 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:318:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'JS::Handle<JS::Value>' for 2nd argument
 0:06.93 ToJSValue(JSContext* aCx, JS::Handle<JS::Value> aArgument,
 0:06.93 ^
 0:06.93 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:327:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'const JS::Heap<JS::Value>' for 2nd argument
 0:06.93 ToJSValue(JSContext* aCx, const JS::Heap<JS::Value>& aArgument,
 0:06.93 ^
 0:06.93 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:336:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'const JS::Rooted<JS::Value>' for 2nd argument
 0:06.93 ToJSValue(JSContext* aCx, const JS::Rooted<JS::Value>& aArgument,
 0:06.93 ^
 0:06.93 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:346:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'const JS::Rooted<JSObject *>' for 2nd argument
 0:06.93 ToJSValue(JSContext* aCx, const JS::Rooted<JSObject*>& aArgument,
 0:06.93 ^
 0:06.93 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:356:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'nsresult' for 2nd argument
 0:06.93 ToJSValue(JSContext* aCx,
 0:06.93 ^
 0:06.93 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:364:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'mozilla::ErrorResult &' for 2nd argument
 0:06.93 ToJSValue(JSContext* aCx,
 0:06.93 ^
 0:06.93 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:393:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'mozilla::dom::Promise &' for 2nd argument
 0:06.93 ToJSValue(JSContext* aCx,
 0:06.93 ^
 0:06.94 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/MediaCapabilitiesBinding.h:49:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'mozilla::dom::MediaDecodingType' for 2nd argument
 0:06.94 ToJSValue(JSContext* aCx, MediaDecodingType aArgument, JS::MutableHandle<JS::Value> aValue);
 0:06.94 ^
 0:06.94 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/MediaCapabilitiesBinding.h:63:1: note: candidate function not viable: no known conversion from 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >' to 'mozilla::dom::MediaEncodingType' for 2nd argument
 0:06.94 ToJSValue(JSContext* aCx, MediaEncodingType aArgument, JS::MutableHandle<JS::Value> aValue);
 0:06.94 ^
 0:06.94 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:44:1: note: candidate template ignored: substitution failure [with T = mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >]: no type named 'Type' in 'mozilla::EnableIf<false, bool>'
 0:06.94 ToJSValue(JSContext* aCx,
 0:06.94 ^
 0:06.94 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:148:1: note: candidate template ignored: substitution failure [with T = const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >]: no type named 'Type' in 'mozilla::EnableIf<false, bool>'
 0:06.94 ToJSValue(JSContext* aCx,
 0:06.94 ^
 0:06.94 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:199:1: note: candidate template ignored: could not match 'nsAutoPtr' against 'UniquePtr'
 0:06.94 ToJSValue(JSContext* aCx,
 0:06.94 ^
 0:06.94 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:219:1: note: candidate template ignored: deduced type 'UniquePtr<...>' of 2nd parameter does not match adjusted type 'UniquePtr<...>' of argument [with T = mozilla::dom::MediaCapabilitiesInfo]
 0:06.94 ToJSValue(JSContext* aCx,
 0:06.94 ^
 0:06.94 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:236:1: note: candidate template ignored: could not match 'TypedArrayCreator' against 'UniquePtr'
 0:06.94 ToJSValue(JSContext* aCx,
 0:06.94 ^
 0:06.95 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:258:1: note: candidate template ignored: substitution failure [with T = const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >]: no type named 'Type' in 'mozilla::EnableIf<false, bool>'
 0:06.95 ToJSValue(JSContext* aCx,
 0:06.95 ^
 0:06.95 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:273:1: note: candidate template ignored: could not match 'nsCOMPtr' against 'UniquePtr'
 0:06.95 ToJSValue(JSContext* aCx,
 0:06.95 ^
 0:06.95 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:282:1: note: candidate template ignored: could not match 'RefPtr' against 'UniquePtr'
 0:06.95 ToJSValue(JSContext* aCx,
 0:06.95 ^
 0:06.95 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:291:1: note: candidate template ignored: could not match 'NonNull' against 'UniquePtr'
 0:06.95 ToJSValue(JSContext* aCx,
 0:06.95 ^
 0:06.95 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:302:1: note: candidate template ignored: substitution failure [with T = mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >]: no type named 'Type' in 'mozilla::EnableIf<false, bool>'
 0:06.95 ToJSValue(JSContext* aCx,
 0:06.95 ^
 0:06.95 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:372:1: note: candidate template ignored: substitution failure [with T = mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >]: no type named 'Type' in 'mozilla::EnableIf<false, bool>'
 0:06.95 ToJSValue(JSContext* aCx,
 0:06.95 ^
 0:06.95 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:384:1: note: candidate template ignored: substitution failure [with T = mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >]: no type named 'Type' in 'mozilla::EnableIf<false, bool>'
 0:06.95 ToJSValue(JSContext* aCx,
 0:06.95 ^
 0:06.95 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:407:1: note: candidate template ignored: could not match 'nsTArray' against 'UniquePtr'
 0:06.95 ToJSValue(JSContext* aCx,
 0:06.95 ^
 0:06.95 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:417:1: note: candidate template ignored: could not match 'FallibleTArray' against 'UniquePtr'
 0:06.95 ToJSValue(JSContext* aCx,
 0:06.96 ^
 0:06.96 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:427:1: note: candidate template ignored: could not match 'const T [N]' against 'const mozilla::UniquePtr<mozilla::dom::MediaCapabilitiesInfo, mozilla::DefaultDelete<mozilla::dom::MediaCapabilitiesInfo> >'
 0:06.96 ToJSValue(JSContext* aCx,
 0:06.96 ^
 0:06.96 /Users/jyavenard/Work/Mozilla/obj-ff-dbg/dist/include/mozilla/dom/ToJSValue.h:437:1: note: candidate function template not viable: requires 4 arguments, but 3 were provided
 0:06.96 ToJSValue(JSContext* aCx,
 0:06.96 ^
 0:06.96 1 error generated.

will look at adding the required specialization in Promise.h

Jean-Yves Avenard [:jya]

Reporter

Comment 12

•

6 years ago

Attached patch . P2. Use forwarding reference in MaybeResolve. r?bz — Details — Splinter Review

This allows move semantics when resolving Promises

MozReview-Commit-ID: EYFd3Abr7Ec

Jean-Yves Avenard [:jya]

Reporter

Updated

•

6 years ago

Attachment #8982798 - Flags: review?(bzbarsky)

Jean-Yves Avenard [:jya]

Reporter

Updated

•

6 years ago

Attachment #8982641 - Attachment is obsolete: true

Jean-Yves Avenard [:jya]

Reporter

Updated

•

6 years ago

Attachment #8982642 - Attachment is obsolete: true

Jean-Yves Avenard [:jya]

Reporter

Updated

•

6 years ago

Attachment #8982643 - Attachment is obsolete: true

Jean-Yves Avenard [:jya]

Reporter

Updated

•

6 years ago

Attachment #8982644 - Attachment is obsolete: true

Jean-Yves Avenard [:jya]

Reporter

Updated

•

6 years ago

Attachment #8982645 - Attachment is obsolete: true

Boris Zbarsky [:bzbarsky]

Assignee

Comment 13

•

6 years ago

Comment on attachment 8982798 [details] [diff] [review]
. P2. Use forwarding reference in MaybeResolve. r?bz

This should probably be in a separate bug, since it can land completely independently of the other change.  (And the huge pastes should probably have been attachments...)

>+    Promise::MaybeResolve(std::forward<T>(aArg));

mozilla::Forward, all 3 places.  Which maybe we can eliminate now if stlport is updated enough, but that should _also_ be a separate bug.

And include mozilla/Move.h, not utility, of course.

r=me with those changes; thank you for fixing this!

Attachment #8982798 - Flags: review?(bzbarsky) → review+

Jean-Yves Avenard [:jya]

Reporter

Comment 14

•

6 years ago

Seeing that we changed all Move to std:move, continuing to use Forward doesn’t make much sense anymore...

Will open another bug.

Jean-Yves Avenard [:jya]

Reporter

Updated

•

6 years ago

Comment 15

•

6 years ago

Comment on attachment 8982769 [details] [diff] [review]
Implement ToJSValue variants for non-refcounted (so owned) non-wrapper-cached DOM objects

Review of attachment 8982769 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/ToJSValue.h
@@ +161,5 @@
> +namespace binding_detail {
> +template<class T>
> +MOZ_MUST_USE
> +typename EnableIf<
> +  IsBaseOf<NonRefcountedDOMObject, T>::value && !IsBaseOf<nsWrapperCache, T>::value,

I wonder if we could just declare that NonRefcountedDOMObject means not wrappercached.

Attachment #8982769 - Flags: review?(peterv) → review+

Boris Zbarsky [:bzbarsky]

Assignee

Comment 16

•

6 years ago

Attached patch Something like this and then kill off the conditions on ToJSValue? — Details — Splinter Review

Attachment #8983559 - Flags: review?(peterv)

Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)

Updated

•

6 years ago

Priority: -- → P2

Peter Van der Beken [:peterv]

Updated

•

6 years ago

Attachment #8983559 - Flags: review?(peterv) → review+

Pulsebot

Comment 17

•

6 years ago

Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/292092eabd6f
Implement ToJSValue variants for non-refcounted (so owned) DOM objects.  r=peterv

Bogdan Tara[:bogdan_tara | bogdant]

Comment 18

•

6 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/292092eabd6f

Status: ASSIGNED → RESOLVED

Closed: 6 years ago

status-firefox62: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla62

Boris Zbarsky [:bzbarsky]

Assignee

Updated

•

6 years ago

Summary: Add ToJSValue specialisation for objects that do not inherit from wrapperCached but implement WrapObject → Add ToJSValue specialisation for objects that do not inherit from wrapperCache but implement WrapObject

Boris Zbarsky [:bzbarsky]

Assignee

Updated

•

6 years ago

Summary: Add ToJSValue specialisation for objects that do not inherit from wrapperCache but implement WrapObject → Add ToJSValue specialisation for objects that do not inherit from nsWrapperCache but implement WrapObject

You need to log in before you can comment on or make changes to this bug.