Affected versions

• Firefox 82.0
• Firefox 83.0a1

Affected platforms

• macOS 11 Beta (20A5395g)

Steps to reproduce

1. Launch Firefox
2. Access Youtube and play any video
3. Click on the Play button from menu bar on the top right corner of the screen

Expected result

• Information about what is currently playing is shown

Actual result

• Only the Firefox's .app name is shown

Regression range

• No regression, as this is a new feature integrated in macOS 11

Additional notes

• The issue can be seen in the attachment

Suggested severity

• S3

:csasca, if you think that's a regression, then could you try to find a regression range in using for example mozregression?

It looks like media control related, not sure if new OS adds new API or supports some old API usage to show that on their new notification center.

Depends on D97066

Depends on D97223

I've finished the implementation, but now got stuck on the build issue, because the machine on the try server is still using 10.11 SDK where MediaPlayer hasn't been introduced yet.

I'm still suffering from the build/link issues, here I'm going to describe the problems I encountered.

1. Use MediaPlayer framework directly (failed)
   As we are still using 10.11 on the try server, which is an older SDK, we are not able to find MediaPlayer framework to link on that SDK version.

2. Declare extern ... MPMediaItemPropertyXXX (failed)
   Then I tried to define something following in MediaPlayerWrapper.h

extern NSString* const _Nullable MPMediaItemPropertyTitle;
extern NSString* const _Nullable MPMediaItemPropertyArtist;
extern NSString* const _Nullable MPMediaItemPropertyAlbumTitle;

On the linking stage, it would show the error of not being able to find those symbols. Those variables are originally defined in MPMediaItem.h, which look like that,

MP_EXTERN NSString * const MPMediaItemPropertyTitle                                                                     // filterable
    MP_API(ios(3.0), tvos(9.0), watchos(5.0), macos(10.12.2));
@property (nonatomic, readonly, nullable) NSString *title MP_API(ios(7.0));

MP_EXTERN NSString * const MPMediaItemPropertyAlbumTitle
    MP_API(ios(3.0), tvos(9.0), watchos(5.0), macos(10.12.2));                                                          // filterable
@property (nonatomic, readonly, nullable) NSString *albumTitle MP_API(ios(7.0));

MP_EXTERN NSString * const MPMediaItemPropertyArtist                                                                    // filterable
    MP_API(ios(3.0), tvos(9.0), watchos(5.0), macos(10.12.2));
@property (nonatomic, readonly, nullable) NSString *artist MP_API(ios(7.0));

So they are also not defined there, because I think MP_EXTERN is a term similar with extern?

3. Declare MPMediaItemPropertyXXX with linking (failed)

So I was thinking about if that could be fixed by manually adding link option, such like [1], then I added following codes,
[1] https://searchfox.org/mozilla-central/rev/7b40f0b246ad0b54975b1525811f2ad599b95f33/toolkit/library/moz.build#75-79

LDFLAGS += ["-Wl,-U,_MPMediaItemPropertyTitle"]
LDFLAGS += ["-Wl,-U,_MPMediaItemPropertyArtist"]
LDFLAGS += ["-Wl,-U,_MPMediaItemPropertyAlbumTitle"]

The result is that, I could pass the build, however, it failed immediately when I ran Firefox, the error is that,

dlopen(/Users/alastor/mozilla-dev/mozilla-central/obj-debug-x86_64-apple-darwin20.1.0/dist/NightlyDebug.app/Contents/MacOS/XUL, 265): Symbol not found: _MPMediaItemPropertyAlbumTitle
  Referenced from: /Users/alastor/mozilla-dev/mozilla-central/obj-debug-x86_64-apple-darwin20.1.0/dist/NightlyDebug.app/Contents/MacOS/XUL
  Expected in: flat namespace
 in /Users/alastor/mozilla-dev/mozilla-central/obj-debug-x86_64-apple-darwin20.1.0/dist/NightlyDebug.app/Contents/MacOS/XUL
Couldn't load XPCOM.

4. import <MediaPlayer/MPMediaItem.h> on 12.2+ (failed)

Then I thought probably I can't simply override the declaration for things using MP_EXTERN, I tried to import the header file directly.

#if defined(MAC_OS_X_VERSION_10_12_2) && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12_2
  #import <MediaPlayer/MPMediaItem.h>
#endif

That allows me to pass build, and I could also start my Firefox without issue. However, when running to the code where first access to those MPMediaItemPropertyXXX, the problem would crash immediately with EXC_BAD_ACCESS. I used lldb to check if they are still dangling, but I can see they all have value, so I don't really understand this error.

(lldb) p MPMediaItemPropertyTitle
warning: `this' is not accessible (substituting 0)
(__NSCFConstantString *) $0 = 0x00007fff8277d3e8 "title"
(lldb) p MPMediaItemPropertyArtist
warning: `this' is not accessible (substituting 0)
(__NSCFConstantString *) $1 = 0x00007fff827802e8 "artist"
(lldb) p MPMediaItemPropertyAlbumTitle
warning: `this' is not accessible (substituting 0)
(__NSCFConstantString *) $2 = 0x00007fff82781ea8 "albumTitle"

5. Declare and define MPMediaItemPropertyXXX (failed only on the try server gTest)

Here I went to my last option, force to redefine them manually in MediaPlayerWrapper.h

#define MPMediaItemPropertyTitle @"title"
#define MPMediaItemPropertyArtist @"artist"
#define MPMediaItemPropertyAlbumTitle @"albumTitle"

Okay, this method allows me to build and run Firefox successfully on both the 10.12+ and the 10.11 machine on the try server. However, it would make my gTest fail on the 10.14 machine on the try server. I can successfully pass my gTest on my local environment, but on the 10.14 machine on the try server, it seems that I can't set the value to MPNowPlayingInfoCenter's nowPlayingInfo....

Here is my implementation, which works well for me on Big Sur. But on the 10.14 machine on the try server, I can see the results in DD are correct, which means I set the value correctly into the local variable nowPlayingInfo. However, the results in DD1 are all empty, which means MPNowPlayingInfoCenter didn't update its nowPlayingInfo. [2]

[2] https://treeherder.mozilla.org/logviewer?job_id=322229528&repo=try&lineNumber=29394

void MediaHardwareKeysEventSourceMacMediaCenter::SetMediaMetadata(
    const dom::MediaMetadataBase& aMetadata) {
  NSMutableDictionary* nowPlayingInfo = [NSMutableDictionary dictionary];
  [nowPlayingInfo setObject:nsCocoaUtils::ToNSString(aMetadata.mTitle)
                     forKey:MPMediaItemPropertyTitle];
  [nowPlayingInfo setObject:nsCocoaUtils::ToNSString(aMetadata.mArtist)
                     forKey:MPMediaItemPropertyArtist];
  [nowPlayingInfo setObject:nsCocoaUtils::ToNSString(aMetadata.mAlbum)
                     forKey:MPMediaItemPropertyAlbumTitle];
  NSLog(@"DD | Title in=%s out=%@", NS_ConvertUTF16toUTF8(aMetadata.mTitle).get(),
        nowPlayingInfo[MPMediaItemPropertyTitle]);
  NSLog(@"DD | Artist=%@", nowPlayingInfo[MPMediaItemPropertyArtist]);
  NSLog(@"DD | Album=%@", nowPlayingInfo[MPMediaItemPropertyAlbumTitle]);
  MPNowPlayingInfoCenter* center =
      (MPNowPlayingInfoCenter*)[mpNowPlayingInfoCenterClass defaultCenter];
  center.nowPlayingInfo = nowPlayingInfo;
  NSLog(@"DD1 | Title=%@", center.nowPlayingInfo[MPMediaItemPropertyTitle]);
  NSLog(@"DD1 | Artist=%@", center.nowPlayingInfo[MPMediaItemPropertyArtist]);
  NSLog(@"DD1 | Album=%@", center.nowPlayingInfo[MPMediaItemPropertyAlbumTitle]);
}

We just upgraded the SDK to 10.12 on the try server (finally!) \o/ So I'm going to test my patches again to see if they work on the try server.

Since upgrading to 10.12, now my patches can build successfully on the try server. However, I still encouter same issue (no.5) as I described in comment8 where I would fail on the gtest on the try server.

I can run the ability of showing metadata and pass the gTest on my local enviroment successfully, but it keeps failling on the try server. From the [1] log result, it seems that I were not able to set nowPlayingInfo on the default MPNowPlayingInfoCenter, but I have no idea why....

In other test cases, we indeed change MPNowPlayingInfoCenter's playback, so I don't understand why we would fail to set nowPlayingInfo.

Hi, Stephen,
I wonder if you have any thought or experience about this kind of error which can help me solve this mysterious problem.
Thank you so much.

This does seem strange indeed. Can you run this with some more logging? For example, could you try to print the entire nowPlayingInfo dictionary via NSLog() using the %@ specifier? Possibly before and after you're trying to set it?

Yes, I've printed them and here is the result. The patch contains delog log is here.

[task 2020-12-09T22:09:39.174Z] 22:09:39     INFO -  2020-12-09 22:09:39.172 firefox[1677:10768] DD | Title in=MediaPlayback out=MediaPlayback
[task 2020-12-09T22:09:39.193Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD | Artist=Firefox
[task 2020-12-09T22:09:39.193Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD | Album=Mozilla
[task 2020-12-09T22:09:39.193Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD | MPMediaItemPropertyTitle=title
[task 2020-12-09T22:09:39.193Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD | MPMediaItemPropertyArtist=artist
[task 2020-12-09T22:09:39.194Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD | MPMediaItemPropertyAlbumTitle=albumTitle
[task 2020-12-09T22:09:39.194Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD1 | Title=
[task 2020-12-09T22:09:39.194Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD1 | Artist=
[task 2020-12-09T22:09:39.194Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD1 | Album=

From above log, you can see that nowPlayingInfo in defaultCenter wasn't updated. But that didn't happen on running either Firefox or gTest on my local 11.0.1 enviorment, which makes me super confused.

I don't know why my previous comment was marked as private. Submitting it again in public:

Could you let me know how I can run your latest patches (and this gtest) on try myself? What exactly would I need to push?

I'm wondering if there is a race somewhere with the newly introduced center.nowPlayingInfo = nil; in MediaHardwareKeysEventSourceMacMediaCenter::EndListeningForEvents in https://phabricator.services.mozilla.com/D97224. The native APIs are asynchronous to a certain extent.

If you want to run it locally, simply apply those two patches and run ./mach gtest MediaHardwareKeysEventSourceMacMediaCenter.\*.

When running gTest on the try server, I use ./mach try chooser and choose gTest under Test Suites on macOS, then gTest should be able to run on the try server. I just pushed two tasks on the try, one for the orignal patch with debug log, and another one is to delete the center.nowPlayingInfo = nil in EndListeningForEvents().

However, EndListeningForEvents() would be called when (1) close the source (2) destroy the source, which are both impossible to happen when we check the assertion, because at the time we check the assertion, the source is still open. So that failure is really myterious to me.

Yes in the beginning I was also considering if those API are actuaaly async, so maybe that is why I failed on checking the nowPlayingInfo immediately after I set its value. However, after I added the debug log, I did see the nowPlayingInfo in the defaultCenterchanged immediately after I set its vaule by checking the printed log.

Thank you so much!

If we really can't find the solution for that, I would probably to remove the test :( but that would be the last choice.

I have been looking into this today and I suspect that the SDK version on try is not sufficiently high to have built-in support for the MediaPlayer library. I am not sure how your build on try succeeded, since my attempts fail with this error:

ld: framework not found MediaPlayer
See: https://treeherder.mozilla.org/logviewer?job_id=324747396&repo=try&lineNumber=47208

My try push: https://treeherder.mozilla.org/jobs?repo=try&revision=11a711d655d58d826c5c115ba2fd9fdc8888d2ca&selectedTaskRun=FGVP9KxWRH2MEnFi8aeMaA.0

This library is expected to be supported in macOS 10.12.1+. I have asked about the specific SDK version that is now on try in bug 1680152 comment 12.

That is weird, if you check my push in comment 15, then they were both using 10.12 SDK for building.

Considering the current tests we have, I think we still have the robust test coverage even if we don't provide gtest for this. I would like to seperate this implementation and gtest (filed bug1683020), and land the implementation first.

I randomly chose three tasks [1] from the try server, and they all used 10.12 SDK, so I'm not sure why your task would build with 10.11 SDk.

[1]
https://treeherder.mozilla.org/logviewer?job_id=324776746&repo=try&lineNumber=34512
https://treeherder.mozilla.org/logviewer?job_id=324768209&repo=try&lineNumber=1184
https://treeherder.mozilla.org/logviewer?job_id=324749219&repo=try&lineNumber=740

Depends on D97223

https://hg.mozilla.org/mozilla-central/rev/5ce98ac27b80
https://hg.mozilla.org/mozilla-central/rev/79cb66d1fe96

I can confirm this fix on Mac OS 11.1 with Nightly v87.0a1 from 2021-01-26 (also from 2021-01-29), but not on Beta v86.0b1 or v86.0b3.
The play icon from the top bar does not appear as it does for nightly builds. Is it supposed to?

What do you mean the play icon doesn't appear? Could you screenshot what you see? I tested that on 86.0b3, which works well.
Thank you.

I have retested today on Beta v86.0b3 and v86.0b4. The functionality was observed to work correctly.
Initially, it was just not appearing for the Beta build, but I could not have reproduced that again.
I deem this bug verified. Sorry about the confusion.