Title: [277481] trunk/Source/WebCore
- Revision
- 277481
- Author
- [email protected]
- Date
- 2021-05-13 21:44:10 -0700 (Thu, 13 May 2021)
Log Message
[Modern Media Controls] REGRESSION(r268308) AirPlay briefly disappears and then reappears when hovering over controls
https://bugs.webkit.org/show_bug.cgi?id=225780
<rdar://problem/77984683>
Reviewed by Eric Carlson.
r268308 adjusted `AVRoutePickerViewTargetPicker::isAvailable`, which is used to control
whether `AVRoutePickerViewTargetPicker` (which uses the `AVRouteDetectorMultipleRoutesDetectedDidChange`
notification and actually stops listening for it in `stopMonitoringPlaybackTargets`) or
`AVOutputDeviceMenuControllerTargetPicker` (which uses ObjC KVO and doesn't actually do
anything in `stopMonitoringPlaybackTargets` when the last JS `"webkitplaybacktargetavailabilitychanged"`
event listener is removed, meaning that WebKit never senda a new value to the WebProcess) is
used. When using `AVRoutePickerViewTargetPicker`, WebKit calls `-[AVRouteDetector setRouteDetectionEnabled:]`
whenever the first JS `"webkitplaybacktargetavailabilitychanged"` event listener is added
(with the argument `YES`) and the last JS `"webkitplaybacktargetavailabilitychanged"` event
listener is removed (with the argument `NO`). In the latter scenario (which is the case with
builtin media controls), `-[AVRouteDetector setRouteDetectionEnabled:]` will dispatch a
`AVRouteDetectorMultipleRoutesDetectedDidChange` notification and mark itself as not having
multiple routes (`-[AVRouteDetector multipleRoutesDetected]`). This work is done in the
UIProcess and the result is sent to the WebProcess, meaning that even though there are no
more JS event listeners WebKit still updates the cached state of whether multiple routes
exist. This means that the next time a JS event listener is added, WebKit will ask the
UIProcess to update (which will re-enable route detection, which will dispatch a `AVRouteDetectorMultipleRoutesDetectedDidChange`
notification) but then immediately dispatch a JS `"webkitplaybacktargetavailabilitychanged"`
event using the cached state. Once the request from the UIProcess comes back, WebKit will
then dispatch *another* JS "webkitplaybacktargetavailabilitychanged" event with the new
non-cached value.
* platform/graphics/avfoundation/objc/AVRoutePickerViewTargetPicker.h:
* platform/graphics/avfoundation/objc/AVRoutePickerViewTargetPicker.mm:
(WebCore::AVRoutePickerViewTargetPicker::startingMonitoringPlaybackTargets):
(WebCore::AVRoutePickerViewTargetPicker::stopMonitoringPlaybackTargets):
(WebCore::AVRoutePickerViewTargetPicker::availableDevicesDidChange):
Add a flag that ignores the next `AVRouteDetectorMultipleRoutesDetectedDidChange`
notification since it's guaranteed to be `false` after `setRouteDetectionEnabled:NO`.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (277480 => 277481)
--- trunk/Source/WebCore/ChangeLog 2021-05-14 04:39:57 UTC (rev 277480)
+++ trunk/Source/WebCore/ChangeLog 2021-05-14 04:44:10 UTC (rev 277481)
@@ -1,3 +1,41 @@
+2021-05-13 Devin Rousso <[email protected]>
+
+ [Modern Media Controls] REGRESSION(r268308) AirPlay briefly disappears and then reappears when hovering over controls
+ https://bugs.webkit.org/show_bug.cgi?id=225780
+ <rdar://problem/77984683>
+
+ Reviewed by Eric Carlson.
+
+ r268308 adjusted `AVRoutePickerViewTargetPicker::isAvailable`, which is used to control
+ whether `AVRoutePickerViewTargetPicker` (which uses the `AVRouteDetectorMultipleRoutesDetectedDidChange`
+ notification and actually stops listening for it in `stopMonitoringPlaybackTargets`) or
+ `AVOutputDeviceMenuControllerTargetPicker` (which uses ObjC KVO and doesn't actually do
+ anything in `stopMonitoringPlaybackTargets` when the last JS `"webkitplaybacktargetavailabilitychanged"`
+ event listener is removed, meaning that WebKit never senda a new value to the WebProcess) is
+ used. When using `AVRoutePickerViewTargetPicker`, WebKit calls `-[AVRouteDetector setRouteDetectionEnabled:]`
+ whenever the first JS `"webkitplaybacktargetavailabilitychanged"` event listener is added
+ (with the argument `YES`) and the last JS `"webkitplaybacktargetavailabilitychanged"` event
+ listener is removed (with the argument `NO`). In the latter scenario (which is the case with
+ builtin media controls), `-[AVRouteDetector setRouteDetectionEnabled:]` will dispatch a
+ `AVRouteDetectorMultipleRoutesDetectedDidChange` notification and mark itself as not having
+ multiple routes (`-[AVRouteDetector multipleRoutesDetected]`). This work is done in the
+ UIProcess and the result is sent to the WebProcess, meaning that even though there are no
+ more JS event listeners WebKit still updates the cached state of whether multiple routes
+ exist. This means that the next time a JS event listener is added, WebKit will ask the
+ UIProcess to update (which will re-enable route detection, which will dispatch a `AVRouteDetectorMultipleRoutesDetectedDidChange`
+ notification) but then immediately dispatch a JS `"webkitplaybacktargetavailabilitychanged"`
+ event using the cached state. Once the request from the UIProcess comes back, WebKit will
+ then dispatch *another* JS "webkitplaybacktargetavailabilitychanged" event with the new
+ non-cached value.
+
+ * platform/graphics/avfoundation/objc/AVRoutePickerViewTargetPicker.h:
+ * platform/graphics/avfoundation/objc/AVRoutePickerViewTargetPicker.mm:
+ (WebCore::AVRoutePickerViewTargetPicker::startingMonitoringPlaybackTargets):
+ (WebCore::AVRoutePickerViewTargetPicker::stopMonitoringPlaybackTargets):
+ (WebCore::AVRoutePickerViewTargetPicker::availableDevicesDidChange):
+ Add a flag that ignores the next `AVRouteDetectorMultipleRoutesDetectedDidChange`
+ notification since it's guaranteed to be `false` after `setRouteDetectionEnabled:NO`.
+
2021-05-13 Wenson Hsieh <[email protected]>
[Cocoa] Plumb data detector results through some platform objects
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/AVRoutePickerViewTargetPicker.h (277480 => 277481)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/AVRoutePickerViewTargetPicker.h 2021-05-14 04:39:57 UTC (rev 277480)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/AVRoutePickerViewTargetPicker.h 2021-05-14 04:44:10 UTC (rev 277481)
@@ -66,6 +66,7 @@
RetainPtr<AVOutputContext> m_outputContext;
RetainPtr<WebAVRoutePickerViewHelper> m_routePickerViewDelegate;
bool m_hadActiveRoute { false };
+ bool m_ignoreNextMultipleRoutesDetectedDidChangeNotification { false };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/AVRoutePickerViewTargetPicker.mm (277480 => 277481)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/AVRoutePickerViewTargetPicker.mm 2021-05-14 04:39:57 UTC (rev 277480)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/AVRoutePickerViewTargetPicker.mm 2021-05-14 04:44:10 UTC (rev 277481)
@@ -137,13 +137,24 @@
void AVRoutePickerViewTargetPicker::startingMonitoringPlaybackTargets()
{
+ m_ignoreNextMultipleRoutesDetectedDidChangeNotification = false;
+
routeDetector().routeDetectionEnabled = YES;
}
void AVRoutePickerViewTargetPicker::stopMonitoringPlaybackTargets()
{
- if (m_routeDetector)
- [m_routeDetector setRouteDetectionEnabled:NO];
+ if (!m_routeDetector)
+ return;
+
+ // `-[AVRouteDetector multipleRoutesDetected]` will always return `NO` if route detection is
+ // disabled and `-[AVRouteDetector setRouteDetectionEnabled:]` will always dispatch a
+ // `AVRouteDetectorMultipleRoutesDetectedDidChange` notification, so ignore the next one in
+ // order to prevent the cached value in the WebProcess from always being `false` when the last
+ // JS `"webkitplaybacktargetavailabilitychanged"` event listener is removed.
+ m_ignoreNextMultipleRoutesDetectedDidChangeNotification = true;
+
+ [m_routeDetector setRouteDetectionEnabled:NO];
}
bool AVRoutePickerViewTargetPicker::externalOutputDeviceAvailable()
@@ -177,6 +188,11 @@
}
void AVRoutePickerViewTargetPicker::availableDevicesDidChange()
{
+ if (m_ignoreNextMultipleRoutesDetectedDidChangeNotification) {
+ m_ignoreNextMultipleRoutesDetectedDidChangeNotification = false;
+ return;
+ }
+
if (client())
client()->availableDevicesChanged();
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes