Skip to content

Commit

Permalink
Revert of Oilpan: Move MediaStreamSource, MediaStreamComponent and Me…
Browse files Browse the repository at this point in the history
…diaStreamDescriptor to oilpan's heap (patchset #5 id:80001 of https://codereview.chromium.org/509933002/)

Reason for revert:
Broke multiple tests in content_unittests.
e.g. https://build.chromium.org/p/chromium.webkit/builders/Linux%20Tests/builds/39151


Original issue's description:
> Oilpan: Move MediaStreamSource, MediaStreamComponent and MediaStreamDescriptor to oilpan's heap
> 
> - MediaStreamSource, MediaStreamComponent and MediaStreamDescriptor must be moved to oilpan's heap in one go because their lifetime are tightly coupled.
> 
> - Removed m_descriptor->setClient(0) from MediaStream's destructor. This is OK because MediaStreamDescriptor has a strong Member back to the Client (i.e., MediaStream).
> 
> - Removed m_component->source()->removeObserver(this) from MediaStreamTrack's destructor. This is OK because this CL made the observers weak (i.e., MediaStreamSource::m_observers is a hash set of weak members to MediaStreamTrack objects).
> 
> - Removed WebMediaStreamTrack::ExtraData::m_owner because it's unused.
> 
> 
> BUG=340522
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181204

[email protected],[email protected],[email protected]
NOTREECHECKS=true
NOTRY=true
BUG=340522

Review URL: https://codereview.chromium.org/534473002

git-svn-id: svn://svn.chromium.org/blink/trunk@181207 bbb929c8-8fbe-4397-9dbb-9b2b20218538
  • Loading branch information
tkent-google committed Sep 2, 2014
1 parent 3f8c215 commit 77a99a6
Show file tree
Hide file tree
Showing 23 changed files with 130 additions and 104 deletions.
7 changes: 3 additions & 4 deletions Source/modules/mediastream/MediaStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,12 @@ MediaStream* MediaStream::create(ExecutionContext* context, const MediaStreamTra
return adoptRefCountedGarbageCollectedWillBeNoop(new MediaStream(context, audioTracks, videoTracks));
}

MediaStream* MediaStream::create(ExecutionContext* context, MediaStreamDescriptor* streamDescriptor)
MediaStream* MediaStream::create(ExecutionContext* context, PassRefPtr<MediaStreamDescriptor> streamDescriptor)
{
return adoptRefCountedGarbageCollectedWillBeNoop(new MediaStream(context, streamDescriptor));
}

MediaStream::MediaStream(ExecutionContext* context, MediaStreamDescriptor* streamDescriptor)
MediaStream::MediaStream(ExecutionContext* context, PassRefPtr<MediaStreamDescriptor> streamDescriptor)
: ContextLifecycleObserver(context)
, m_stopped(false)
, m_descriptor(streamDescriptor)
Expand Down Expand Up @@ -150,6 +150,7 @@ MediaStream::MediaStream(ExecutionContext* context, const MediaStreamTrackVector

MediaStream::~MediaStream()
{
m_descriptor->setClient(0);
}

bool MediaStream::ended() const
Expand Down Expand Up @@ -394,9 +395,7 @@ void MediaStream::trace(Visitor* visitor)
visitor->trace(m_audioTracks);
visitor->trace(m_videoTracks);
visitor->trace(m_scheduledEvents);
visitor->trace(m_descriptor);
EventTargetWithInlineData::trace(visitor);
MediaStreamDescriptorClient::trace(visitor);
}

} // namespace blink
8 changes: 4 additions & 4 deletions Source/modules/mediastream/MediaStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ class MediaStream FINAL
, public EventTargetWithInlineData
, public ContextLifecycleObserver {
DEFINE_EVENT_TARGET_REFCOUNTING_WILL_BE_REMOVED(RefCountedGarbageCollectedWillBeGarbageCollectedFinalized<MediaStream>);
USING_GARBAGE_COLLECTED_MIXIN(MediaStream);
WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(MediaStream);
public:
static MediaStream* create(ExecutionContext*);
static MediaStream* create(ExecutionContext*, MediaStream*);
static MediaStream* create(ExecutionContext*, const MediaStreamTrackVector&);
static MediaStream* create(ExecutionContext*, MediaStreamDescriptor*);
static MediaStream* create(ExecutionContext*, PassRefPtr<MediaStreamDescriptor>);
virtual ~MediaStream();

// DEPRECATED
Expand Down Expand Up @@ -90,7 +90,7 @@ class MediaStream FINAL
virtual void trace(Visitor*) OVERRIDE;

private:
MediaStream(ExecutionContext*, MediaStreamDescriptor*);
MediaStream(ExecutionContext*, PassRefPtr<MediaStreamDescriptor>);
MediaStream(ExecutionContext*, const MediaStreamTrackVector& audioTracks, const MediaStreamTrackVector& videoTracks);

// ContextLifecycleObserver
Expand All @@ -107,7 +107,7 @@ class MediaStream FINAL

MediaStreamTrackVector m_audioTracks;
MediaStreamTrackVector m_videoTracks;
Member<MediaStreamDescriptor> m_descriptor;
RefPtr<MediaStreamDescriptor> m_descriptor;

Timer<MediaStream> m_scheduledEventTimer;
WillBeHeapVector<RefPtrWillBeMember<Event> > m_scheduledEvents;
Expand Down
3 changes: 1 addition & 2 deletions Source/modules/mediastream/MediaStreamRegistry.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#define MediaStreamRegistry_h

#include "core/html/URLRegistry.h"
#include "platform/heap/Handle.h"
#include "wtf/HashMap.h"
#include "wtf/PassRefPtr.h"
#include "wtf/text/StringHash.h"
Expand All @@ -51,7 +50,7 @@ class MediaStreamRegistry FINAL : public URLRegistry {

private:
MediaStreamRegistry();
PersistentHeapHashMap<String, Member<MediaStreamDescriptor> > m_streamDescriptors;
HashMap<String, RefPtr<MediaStreamDescriptor> > m_streamDescriptors;
};

} // namespace blink
Expand Down
9 changes: 4 additions & 5 deletions Source/modules/mediastream/MediaStreamTrack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ MediaStreamTrack::MediaStreamTrack(ExecutionContext* context, MediaStreamCompone

MediaStreamTrack::~MediaStreamTrack()
{
m_component->source()->removeObserver(this);
}

String MediaStreamTrack::kind() const
Expand Down Expand Up @@ -146,9 +147,9 @@ void MediaStreamTrack::stopTrack(ExceptionState& exceptionState)

MediaStreamTrack* MediaStreamTrack::clone(ExecutionContext* context)
{
MediaStreamComponent* clonedComponent = MediaStreamComponent::create(component()->source());
MediaStreamTrack* clonedTrack = MediaStreamTrack::create(context, clonedComponent);
MediaStreamCenter::instance().didCreateMediaStreamTrack(clonedComponent);
RefPtr<MediaStreamComponent> clonedComponent = MediaStreamComponent::create(component()->source());
MediaStreamTrack* clonedTrack = MediaStreamTrack::create(context, clonedComponent.get());
MediaStreamCenter::instance().didCreateMediaStreamTrack(clonedComponent.get());
return clonedTrack;
}

Expand Down Expand Up @@ -229,9 +230,7 @@ ExecutionContext* MediaStreamTrack::executionContext() const
void MediaStreamTrack::trace(Visitor* visitor)
{
visitor->trace(m_registeredMediaStreams);
visitor->trace(m_component);
EventTargetWithInlineData::trace(visitor);
MediaStreamSource::Observer::trace(visitor);
}

} // namespace blink
4 changes: 2 additions & 2 deletions Source/modules/mediastream/MediaStreamTrack.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class MediaStreamTrack FINAL
, public EventTargetWithInlineData
, public MediaStreamSource::Observer {
DEFINE_EVENT_TARGET_REFCOUNTING_WILL_BE_REMOVED(RefCountedGarbageCollectedWillBeGarbageCollectedFinalized<MediaStreamTrack>);
USING_GARBAGE_COLLECTED_MIXIN(MediaStreamTrack);
WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(MediaStreamTrack);
public:
static MediaStreamTrack* create(ExecutionContext*, MediaStreamComponent*);
virtual ~MediaStreamTrack();
Expand Down Expand Up @@ -99,7 +99,7 @@ class MediaStreamTrack FINAL
HeapHashSet<Member<MediaStream> > m_registeredMediaStreams;
bool m_isIteratingRegisteredMediaStreams;
bool m_stopped;
Member<MediaStreamComponent> m_component;
RefPtr<MediaStreamComponent> m_component;
};

typedef HeapVector<Member<MediaStreamTrack> > MediaStreamTrackVector;
Expand Down
1 change: 0 additions & 1 deletion Source/modules/mediastream/RTCStatsRequestImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ void RTCStatsRequestImpl::clear()
void RTCStatsRequestImpl::trace(Visitor* visitor)
{
visitor->trace(m_successCallback);
visitor->trace(m_component);
visitor->trace(m_requester);
RTCStatsRequest::trace(visitor);
}
Expand Down
2 changes: 1 addition & 1 deletion Source/modules/mediastream/RTCStatsRequestImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class RTCStatsRequestImpl FINAL : public RTCStatsRequest, public ActiveDOMObject
void clear();

OwnPtrWillBeMember<RTCStatsCallback> m_successCallback;
Member<MediaStreamComponent> m_component;
RefPtr<MediaStreamComponent> m_component;
Member<RTCPeerConnection> m_requester;
};

Expand Down
2 changes: 1 addition & 1 deletion Source/modules/mediastream/UserMediaRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ void UserMediaRequest::start()
m_controller->requestUserMedia(this);
}

void UserMediaRequest::succeed(MediaStreamDescriptor* streamDescriptor)
void UserMediaRequest::succeed(PassRefPtr<MediaStreamDescriptor> streamDescriptor)
{
if (!executionContext())
return;
Expand Down
2 changes: 1 addition & 1 deletion Source/modules/mediastream/UserMediaRequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class UserMediaRequest FINAL : public GarbageCollectedFinalized<UserMediaRequest

void start();

void succeed(MediaStreamDescriptor*);
void succeed(PassRefPtr<MediaStreamDescriptor>);
void failPermissionDenied(const String& message);
void failConstraint(const String& constraintName, const String& message);
void failUASpecific(const String& name, const String& message, const String& constraintName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ void MediaStreamAudioDestinationNode::dispose()

void MediaStreamAudioDestinationNode::trace(Visitor* visitor)
{
visitor->trace(m_source);
visitor->trace(m_stream);
AudioBasicInspectorNode::trace(visitor);
}
Expand Down
2 changes: 1 addition & 1 deletion Source/modules/webaudio/MediaStreamAudioDestinationNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class MediaStreamAudioDestinationNode FINAL : public AudioBasicInspectorNode {
virtual bool propagatesSilence() const OVERRIDE { return false; }

Member<MediaStream> m_stream;
Member<MediaStreamSource> m_source;
RefPtr<MediaStreamSource> m_source;
RefPtr<AudioBus> m_mixBus;
};

Expand Down
12 changes: 11 additions & 1 deletion Source/platform/exported/WebMediaStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ class ExtraDataContainer : public MediaStreamDescriptor::ExtraData {

} // namespace

WebMediaStream::WebMediaStream(const PassRefPtr<MediaStreamDescriptor>& mediaStreamDescriptor)
: m_private(mediaStreamDescriptor)
{
}

WebMediaStream::WebMediaStream(MediaStreamDescriptor* mediaStreamDescriptor)
: m_private(mediaStreamDescriptor)
{
Expand Down Expand Up @@ -111,12 +116,17 @@ void WebMediaStream::removeTrack(const WebMediaStreamTrack& track)
m_private->removeRemoteTrack(track);
}

WebMediaStream& WebMediaStream::operator=(MediaStreamDescriptor* mediaStreamDescriptor)
WebMediaStream& WebMediaStream::operator=(const PassRefPtr<MediaStreamDescriptor>& mediaStreamDescriptor)
{
m_private = mediaStreamDescriptor;
return *this;
}

WebMediaStream::operator PassRefPtr<MediaStreamDescriptor>() const
{
return m_private.get();
}

WebMediaStream::operator MediaStreamDescriptor*() const
{
return m_private.get();
Expand Down
7 changes: 6 additions & 1 deletion Source/platform/exported/WebMediaStreamSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ void WebMediaStreamSource::ExtraData::setOwner(MediaStreamSource* owner)
m_owner = owner;
}

WebMediaStreamSource::WebMediaStreamSource(MediaStreamSource* mediaStreamSource)
WebMediaStreamSource::WebMediaStreamSource(const PassRefPtr<MediaStreamSource>& mediaStreamSource)
: m_private(mediaStreamSource)
{
}
Expand All @@ -90,6 +90,11 @@ void WebMediaStreamSource::reset()
m_private.reset();
}

WebMediaStreamSource::operator PassRefPtr<MediaStreamSource>() const
{
return m_private.get();
}

WebMediaStreamSource::operator MediaStreamSource*() const
{
return m_private.get();
Expand Down
26 changes: 26 additions & 0 deletions Source/platform/exported/WebMediaStreamTrack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,23 @@ class ExtraDataContainer : public MediaStreamComponent::ExtraData {

} // namespace

WebMediaStreamTrack WebMediaStreamTrack::ExtraData::owner()
{
ASSERT(m_owner);
return WebMediaStreamTrack(m_owner);
}

void WebMediaStreamTrack::ExtraData::setOwner(MediaStreamComponent* owner)
{
ASSERT(!m_owner);
m_owner = owner;
}

WebMediaStreamTrack::WebMediaStreamTrack(PassRefPtr<MediaStreamComponent> mediaStreamComponent)
: m_private(mediaStreamComponent)
{
}

WebMediaStreamTrack::WebMediaStreamTrack(MediaStreamComponent* mediaStreamComponent)
: m_private(mediaStreamComponent)
{
Expand All @@ -75,6 +92,11 @@ void WebMediaStreamTrack::reset()
m_private.reset();
}

WebMediaStreamTrack::operator PassRefPtr<MediaStreamComponent>() const
{
return m_private.get();
}

WebMediaStreamTrack::operator MediaStreamComponent*() const
{
return m_private.get();
Expand Down Expand Up @@ -109,6 +131,10 @@ WebMediaStreamTrack::ExtraData* WebMediaStreamTrack::extraData() const
void WebMediaStreamTrack::setExtraData(ExtraData* extraData)
{
ASSERT(!m_private.isNull());

if (extraData)
extraData->setOwner(m_private.get());

m_private->setExtraData(adoptPtr(new ExtraDataContainer(adoptPtr(extraData))));
}

Expand Down
15 changes: 5 additions & 10 deletions Source/platform/mediastream/MediaStreamComponent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,17 @@

namespace blink {

MediaStreamComponent* MediaStreamComponent::create(MediaStreamSource* source)
PassRefPtr<MediaStreamComponent> MediaStreamComponent::create(PassRefPtr<MediaStreamSource> source)
{
return new MediaStreamComponent(createCanonicalUUIDString(), source);
return adoptRef(new MediaStreamComponent(createCanonicalUUIDString(), source));
}

MediaStreamComponent* MediaStreamComponent::create(const String& id, MediaStreamSource* source)
PassRefPtr<MediaStreamComponent> MediaStreamComponent::create(const String& id, PassRefPtr<MediaStreamSource> source)
{
return new MediaStreamComponent(id, source);
return adoptRef(new MediaStreamComponent(id, source));
}

MediaStreamComponent::MediaStreamComponent(const String& id, MediaStreamSource* source)
MediaStreamComponent::MediaStreamComponent(const String& id, PassRefPtr<MediaStreamSource> source)
: m_source(source)
, m_id(id)
, m_enabled(true)
Expand Down Expand Up @@ -88,10 +88,5 @@ void MediaStreamComponent::AudioSourceProviderImpl::provideInput(AudioBus* bus,
}
#endif // #if ENABLE(WEB_AUDIO)

void MediaStreamComponent::trace(Visitor* visitor)
{
visitor->trace(m_source);
}

} // namespace blink

16 changes: 7 additions & 9 deletions Source/platform/mediastream/MediaStreamComponent.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@
#define MediaStreamComponent_h

#include "platform/audio/AudioSourceProvider.h"
#include "platform/heap/Handle.h"
#include "wtf/PassOwnPtr.h"
#include "wtf/PassRefPtr.h"
#include "wtf/RefCounted.h"
#include "wtf/ThreadingPrimitives.h"
#include "wtf/text/WTFString.h"

Expand All @@ -45,15 +45,15 @@ class MediaStreamDescriptor;
class MediaStreamSource;
class WebAudioSourceProvider;

class PLATFORM_EXPORT MediaStreamComponent FINAL : public GarbageCollectedFinalized<MediaStreamComponent> {
class PLATFORM_EXPORT MediaStreamComponent : public RefCounted<MediaStreamComponent> {
public:
class ExtraData {
public:
virtual ~ExtraData() { }
};

static MediaStreamComponent* create(MediaStreamSource*);
static MediaStreamComponent* create(const String& id, MediaStreamSource*);
static PassRefPtr<MediaStreamComponent> create(PassRefPtr<MediaStreamSource>);
static PassRefPtr<MediaStreamComponent> create(const String& id, PassRefPtr<MediaStreamSource>);

MediaStreamSource* source() const { return m_source.get(); }

Expand All @@ -70,10 +70,8 @@ class PLATFORM_EXPORT MediaStreamComponent FINAL : public GarbageCollectedFinali
ExtraData* extraData() const { return m_extraData.get(); }
void setExtraData(PassOwnPtr<ExtraData> extraData) { m_extraData = extraData; }

void trace(Visitor*);

private:
MediaStreamComponent(const String& id, MediaStreamSource*);
MediaStreamComponent(const String& id, PassRefPtr<MediaStreamSource>);

#if ENABLE(WEB_AUDIO)
// AudioSourceProviderImpl wraps a WebAudioSourceProvider::provideInput()
Expand Down Expand Up @@ -102,14 +100,14 @@ class PLATFORM_EXPORT MediaStreamComponent FINAL : public GarbageCollectedFinali
AudioSourceProviderImpl m_sourceProvider;
#endif // ENABLE(WEB_AUDIO)

Member<MediaStreamSource> m_source;
RefPtr<MediaStreamSource> m_source;
String m_id;
bool m_enabled;
bool m_muted;
OwnPtr<ExtraData> m_extraData;
};

typedef HeapVector<Member<MediaStreamComponent> > MediaStreamComponentVector;
typedef Vector<RefPtr<MediaStreamComponent> > MediaStreamComponentVector;

} // namespace blink

Expand Down
Loading

0 comments on commit 77a99a6

Please sign in to comment.