1869053 - Modernize mozilla::mscom::AgileReference

Status: RESOLVED FIXED

(Core :: IPC: MSCOM, task)

Description

[Ray Kraesig [:rkraesig]]

AgileReference has an alternate implementation via the global interface table that is no longer needed, since we no longer support versions of Windows prior to the introduction of RoGetAgileReference.

Comment 1

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1869053 - [1/5] modernize our `mscom::AgileReference` class r?handyman!

As we no longer support Windows versions without RoGetAgileReference,
we can drop the old global-interface-table manipulation version of this
code.

Comment 2

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1869053 - [2/5] eliminate all implicit conversions between RefPtr and AgileReference r?handyman!

Remove all of the code in RefPtr and AgileReference that allowed
for implicitly converting between the two in via constructors or the
assignment operator.

Replace these with a slightly-less-convenient-but-substantially-more-
explicit Resolve function family.

(This also eliminates the dependency that class RefPtr had on
class AgileReference.)

Depends on D196364

Comment 3

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1869053 - [3/5] simplify use of AgileReference in IAudioSessionControl r?handyman!

The documentation for AgileReference previously suggested instantiating
it within a UniquePtr. However, this isn't necessary or helpful in most
contexts.

Streamline the one place where this was done.

Depends on D196365

Comment 4

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1869053 - [4/5] remove EnsureMTA::CreateInstance r?handyman!

This function has not been used since its introduction in 2019.

Depends on D196366

Comment 5

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1869053 - [5/5] templatize and further simplify AgileReference r?handyman!

Lift aIid to compile-time, as a template parameter InterfaceT. This
simplifies the common case for using Resolve(), where the desired and
supplied interfaces are the same. (For the as-yet-unattested case where
they're not, retain the old functionality by means of a small family of
ResolveAs() functions.)

Additionally, to eliminate a swath of custom logic and magic-number
choices surrounding mHResult, eliminate mHResult itself as well.
Instead, since its value was derived from the creation of the underlying
IAgileReference, any callers that might care can acquire it as an
additional return value from a named-constructor function.

These collectively trim AgileReference's footprint down to a single
RefPtr, with all its special member functions having only default
implementations.

Depends on D196512

Comment 6

[Ray Kraesig [:rkraesig]]

Rather than refactor it to work with a templatized AgileInterface, D196512 removes EnsureMTA::CreateInstance, which has never been used.

I don't think it generally should be, at least with the interface it has. If the caller can await on a Promise for the AgileInterface's creation, it can probably also await on a Promise for calling methods on the object in question. Moreover, invoking a method on an MTA-sourced AgileInterface is approximately equivalent to calling EnsureMTA::EnsureMTA() directly, blocking as we do so; most callers would probably be better off awaiting on such a work promise, rather than suffering mini-stalls after the initial await.

(This explanation extracted and paraphrased from the original commit summary for D196512.)

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6addaa9e88be
[1/5] modernize our `mscom::AgileReference` class  r=handyman
https://hg.mozilla.org/integration/autoland/rev/6ab85972356d
[2/5] eliminate all implicit conversions between RefPtr and AgileReference  r=handyman,win-reviewers
https://hg.mozilla.org/integration/autoland/rev/59d9b136a3f2
[3/5] simplify use of AgileReference in IAudioSessionControl  r=handyman,win-reviewers
https://hg.mozilla.org/integration/autoland/rev/687e7683752c
[4/5] remove EnsureMTA::CreateInstance  r=handyman
https://hg.mozilla.org/integration/autoland/rev/0a1ec143adbc
[5/5] templatize and further simplify AgileReference  r=handyman,win-reviewers

Comment 8

[Sandor Molnar[:smolnar]]

Backed out for causing build bustages

Backout link: https://hg.mozilla.org/integration/autoland/rev/c12f6936509f05b3caad77d4cb4b783a38761897

Push with failures

Failure log -> ERROR - ld.lld: error: undefined symbol: __Z19RoGetAgileReference21AgileReferenceOptionsRK5_GUIDP8IUnknownPP15IAgileReference@16

Comment 9

[Ray Kraesig [:rkraesig]]

Failure to link on MinGW, as it doesn't know about RoGetAgileReference.

Per conversation on Matrix, that is fixable — but, to unblock this patch, we'll just link it dynamically on MinGW builds for now.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f4b4c654ebb
[1/5] modernize our `mscom::AgileReference` class  r=handyman
https://hg.mozilla.org/integration/autoland/rev/4d3f0b4a6107
[2/5] eliminate all implicit conversions between RefPtr and AgileReference  r=handyman,win-reviewers
https://hg.mozilla.org/integration/autoland/rev/93c92f80d81f
[3/5] simplify use of AgileReference in IAudioSessionControl  r=handyman,win-reviewers
https://hg.mozilla.org/integration/autoland/rev/9346eb3ea0e4
[4/5] remove EnsureMTA::CreateInstance  r=handyman
https://hg.mozilla.org/integration/autoland/rev/19b846ec490c
[5/5] templatize and further simplify AgileReference  r=handyman,win-reviewers

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/2f4b4c654ebb
https://hg.mozilla.org/mozilla-central/rev/4d3f0b4a6107
https://hg.mozilla.org/mozilla-central/rev/93c92f80d81f
https://hg.mozilla.org/mozilla-central/rev/9346eb3ea0e4
https://hg.mozilla.org/mozilla-central/rev/19b846ec490c