Title: [238181] trunk/Source/WebCore
- Revision
- 238181
- Author
- you...@apple.com
- Date
- 2018-11-14 09:50:36 -0800 (Wed, 14 Nov 2018)
Log Message
Allow to remove MediaStreamPrivate observers when iterating over observers
https://bugs.webkit.org/show_bug.cgi?id=187256
Reviewed by Eric Carlson.
Migrate the observer list from a Vector to a HashSet.
This is more robust to multiple observing and keeping of order of observers is not required.
Copy the set of observers to a vector before iterating over it.
This allows to remove an observer while iterating, which is now used in UserMediaRequest.
Covered by existing tests.
* Modules/mediastream/UserMediaRequest.cpp:
(WebCore::UserMediaRequest::mediaStreamIsReady):
* platform/mediastream/MediaStreamPrivate.cpp:
(WebCore::MediaStreamPrivate::addObserver):
(WebCore::MediaStreamPrivate::removeObserver):
(WebCore::MediaStreamPrivate::forEachObserver const):
(WebCore::MediaStreamPrivate::updateActiveState):
(WebCore::MediaStreamPrivate::addTrack):
(WebCore::MediaStreamPrivate::removeTrack):
(WebCore::MediaStreamPrivate::characteristicsChanged):
* platform/mediastream/MediaStreamPrivate.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (238180 => 238181)
--- trunk/Source/WebCore/ChangeLog 2018-11-14 17:35:02 UTC (rev 238180)
+++ trunk/Source/WebCore/ChangeLog 2018-11-14 17:50:36 UTC (rev 238181)
@@ -1,5 +1,31 @@
2018-11-14 Youenn Fablet <you...@apple.com>
+ Allow to remove MediaStreamPrivate observers when iterating over observers
+ https://bugs.webkit.org/show_bug.cgi?id=187256
+
+ Reviewed by Eric Carlson.
+
+ Migrate the observer list from a Vector to a HashSet.
+ This is more robust to multiple observing and keeping of order of observers is not required.
+ Copy the set of observers to a vector before iterating over it.
+ This allows to remove an observer while iterating, which is now used in UserMediaRequest.
+
+ Covered by existing tests.
+
+ * Modules/mediastream/UserMediaRequest.cpp:
+ (WebCore::UserMediaRequest::mediaStreamIsReady):
+ * platform/mediastream/MediaStreamPrivate.cpp:
+ (WebCore::MediaStreamPrivate::addObserver):
+ (WebCore::MediaStreamPrivate::removeObserver):
+ (WebCore::MediaStreamPrivate::forEachObserver const):
+ (WebCore::MediaStreamPrivate::updateActiveState):
+ (WebCore::MediaStreamPrivate::addTrack):
+ (WebCore::MediaStreamPrivate::removeTrack):
+ (WebCore::MediaStreamPrivate::characteristicsChanged):
+ * platform/mediastream/MediaStreamPrivate.h:
+
+2018-11-14 Youenn Fablet <you...@apple.com>
+
Calling removeTrack on different RTCPeerConnection should throw InvalidAccessError
https://bugs.webkit.org/show_bug.cgi?id=191603
Modified: trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp (238180 => 238181)
--- trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp 2018-11-14 17:35:02 UTC (rev 238180)
+++ trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp 2018-11-14 17:50:36 UTC (rev 238181)
@@ -365,8 +365,7 @@
RELEASE_LOG(MediaStream, "UserMediaRequest::mediaStreamIsReady");
stream->document()->setHasCaptureMediaStreamTrack();
m_promise.resolve(WTFMove(stream));
- // We are in an observer iterator loop, we do not want to change the observers within this loop.
- callOnMainThread([stream = WTFMove(m_pendingActivationMediaStream)] { });
+ m_pendingActivationMediaStream = nullptr;
}
void UserMediaRequest::mediaStreamDidFail(RealtimeMediaSource::Type type)
Modified: trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp (238180 => 238181)
--- trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp 2018-11-14 17:35:02 UTC (rev 238180)
+++ trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp 2018-11-14 17:50:36 UTC (rev 238181)
@@ -84,16 +84,23 @@
void MediaStreamPrivate::addObserver(MediaStreamPrivate::Observer& observer)
{
- m_observers.append(&observer);
+ m_observers.add(&observer);
}
void MediaStreamPrivate::removeObserver(MediaStreamPrivate::Observer& observer)
{
- size_t pos = m_observers.find(&observer);
- if (pos != notFound)
- m_observers.remove(pos);
+ m_observers.remove(&observer);
}
+void MediaStreamPrivate::forEachObserver(const WTF::Function<void(Observer&)>& apply) const
+{
+ for (auto* observer : copyToVector(m_observers)) {
+ if (!m_observers.contains(observer))
+ continue;
+ apply(*observer);
+ }
+}
+
MediaStreamTrackPrivateVector MediaStreamPrivate::tracks() const
{
return copyToVector(m_trackSet.values());
@@ -118,8 +125,9 @@
m_isActive = newActiveState;
if (notifyClientOption == NotifyClientOption::Notify) {
- for (auto& observer : m_observers)
- observer->activeStatusChanged();
+ forEachObserver([](auto& observer) {
+ observer.activeStatusChanged();
+ });
}
}
@@ -132,8 +140,9 @@
m_trackSet.add(track->id(), track);
if (notifyClientOption == NotifyClientOption::Notify) {
- for (auto& observer : m_observers)
- observer->didAddTrack(*track.get());
+ forEachObserver([&track](auto& observer) {
+ observer.didAddTrack(*track.get());
+ });
}
updateActiveState(notifyClientOption);
@@ -148,8 +157,9 @@
track.removeObserver(*this);
if (notifyClientOption == NotifyClientOption::Notify) {
- for (auto& observer : m_observers)
- observer->didRemoveTrack(track);
+ forEachObserver([&track](auto& observer) {
+ observer.didRemoveTrack(track);
+ });
}
updateActiveState(NotifyClientOption::Notify);
@@ -256,8 +266,9 @@
void MediaStreamPrivate::characteristicsChanged()
{
- for (auto& observer : m_observers)
- observer->characteristicsChanged();
+ forEachObserver([](auto& observer) {
+ observer.characteristicsChanged();
+ });
}
void MediaStreamPrivate::trackMutedChanged(MediaStreamTrackPrivate&)
Modified: trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.h (238180 => 238181)
--- trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.h 2018-11-14 17:35:02 UTC (rev 238180)
+++ trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.h 2018-11-14 17:50:36 UTC (rev 238181)
@@ -117,8 +117,9 @@
void updateActiveVideoTrack();
void scheduleDeferredTask(Function<void ()>&&);
+ void forEachObserver(const WTF::Function<void(Observer&)>&) const;
- Vector<Observer*> m_observers;
+ HashSet<Observer*> m_observers;
String m_id;
MediaStreamTrackPrivate* m_activeVideoTrack { nullptr };
HashMap<String, RefPtr<MediaStreamTrackPrivate>> m_trackSet;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
