Commit f02562ee authored by Nate Chapin's avatar Nate Chapin Committed by Commit Bot

Make AudioRendererSinkCache::FrameObserver a window supplement

It's effecitvely per-window as is, and the current design might have
problems in a case where the cache is used from multiple windows that
were associated with the same frame.

Change-Id: Idad13fa9ad91f77be3c0feae6370a0ea396db7c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2516975Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarGuido Urdaneta <guidou@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824512}
parent e687fb66
...@@ -29,48 +29,34 @@ namespace blink { ...@@ -29,48 +29,34 @@ namespace blink {
AudioRendererSinkCache* AudioRendererSinkCache::instance_ = nullptr; AudioRendererSinkCache* AudioRendererSinkCache::instance_ = nullptr;
class AudioRendererSinkCache::FrameObserver final class AudioRendererSinkCache::WindowObserver final
: public GarbageCollected<AudioRendererSinkCache::FrameObserver>, : public GarbageCollected<AudioRendererSinkCache::WindowObserver>,
public Supplement<LocalFrame>, public Supplement<LocalDOMWindow>,
public ExecutionContextLifecycleObserver { public ExecutionContextLifecycleObserver {
public: public:
static const char kSupplementName[]; static const char kSupplementName[];
static FrameObserver* From(LocalFrame& frame) {
return Supplement<LocalFrame>::From<FrameObserver>(frame);
}
explicit FrameObserver(LocalFrame& frame) explicit WindowObserver(LocalDOMWindow& window)
: Supplement<LocalFrame>(frame), : Supplement<LocalDOMWindow>(window),
ExecutionContextLifecycleObserver(frame.DomWindow()) {} ExecutionContextLifecycleObserver(&window) {}
~FrameObserver() { DCHECK(dropped_frame_cached_); } ~WindowObserver() = default;
void Trace(Visitor* visitor) const final { void Trace(Visitor* visitor) const final {
Supplement<LocalFrame>::Trace(visitor); Supplement<LocalDOMWindow>::Trace(visitor);
ExecutionContextLifecycleObserver::Trace(visitor); ExecutionContextLifecycleObserver::Trace(visitor);
} }
// ExecutionContextLifecycleObserver implementation. // ExecutionContextLifecycleObserver implementation.
void ContextDestroyed() override { DropFrameCache(); } void ContextDestroyed() override {
if (auto* cache_instance = AudioRendererSinkCache::instance_)
private: cache_instance->DropSinksForFrame(DomWindow()->GetLocalFrameToken());
void DropFrameCache() {
dropped_frame_cached_ = true;
if (!AudioRendererSinkCache::instance_)
return;
if (!GetSupplementable())
return;
LocalFrameToken frame_token = GetSupplementable()->GetLocalFrameToken();
AudioRendererSinkCache::instance_->DropSinksForFrame(frame_token);
} }
bool dropped_frame_cached_ = false; DISALLOW_COPY_AND_ASSIGN(WindowObserver);
DISALLOW_COPY_AND_ASSIGN(FrameObserver);
}; };
const char AudioRendererSinkCache::FrameObserver::kSupplementName[] = const char AudioRendererSinkCache::WindowObserver::kSupplementName[] =
"AudioRendererSinkCache::FrameObserver"; "AudioRendererSinkCache::WindowObserver";
namespace { namespace {
...@@ -105,12 +91,11 @@ struct AudioRendererSinkCache::CacheEntry { ...@@ -105,12 +91,11 @@ struct AudioRendererSinkCache::CacheEntry {
}; };
// static // static
void AudioRendererSinkCache::InstallFrameObserver(LocalFrame& frame) { void AudioRendererSinkCache::InstallWindowObserver(LocalDOMWindow& window) {
if (AudioRendererSinkCache::FrameObserver::From(frame)) if (Supplement<LocalDOMWindow>::From<WindowObserver>(window))
return; return;
Supplement<LocalFrame>::ProvideTo( Supplement<LocalDOMWindow>::ProvideTo(
frame, window, MakeGarbageCollected<WindowObserver>(window));
MakeGarbageCollected<AudioRendererSinkCache::FrameObserver>(frame));
} }
AudioRendererSinkCache::AudioRendererSinkCache( AudioRendererSinkCache::AudioRendererSinkCache(
......
...@@ -26,7 +26,7 @@ class AudioRendererSink; ...@@ -26,7 +26,7 @@ class AudioRendererSink;
namespace blink { namespace blink {
class LocalFrame; class LocalDOMWindow;
// Caches AudioRendererSink instances, provides them to the clients for usage, // Caches AudioRendererSink instances, provides them to the clients for usage,
// tracks their used/unused state, reuses them to obtain output device // tracks their used/unused state, reuses them to obtain output device
...@@ -34,7 +34,7 @@ class LocalFrame; ...@@ -34,7 +34,7 @@ class LocalFrame;
// Must live on the main render thread. Thread safe. // Must live on the main render thread. Thread safe.
class MODULES_EXPORT AudioRendererSinkCache { class MODULES_EXPORT AudioRendererSinkCache {
public: public:
class FrameObserver; class WindowObserver;
// Callback to be used for AudioRendererSink creation // Callback to be used for AudioRendererSink creation
using CreateSinkCallback = using CreateSinkCallback =
...@@ -42,9 +42,9 @@ class MODULES_EXPORT AudioRendererSinkCache { ...@@ -42,9 +42,9 @@ class MODULES_EXPORT AudioRendererSinkCache {
const LocalFrameToken& frame_token, const LocalFrameToken& frame_token,
const media::AudioSinkParameters& params)>; const media::AudioSinkParameters& params)>;
// If called, the cache will drop sinks belonging to the specified frame on // If called, the cache will drop sinks belonging to the specified window on
// navigation. // navigation.
static void InstallFrameObserver(LocalFrame& frame); static void InstallWindowObserver(LocalDOMWindow&);
// |cleanup_task_runner| will be used to delete sinks when they are unused, // |cleanup_task_runner| will be used to delete sinks when they are unused,
// AudioRendererSinkCache must outlive any tasks posted to it. Since // AudioRendererSinkCache must outlive any tasks posted to it. Since
...@@ -67,7 +67,7 @@ class MODULES_EXPORT AudioRendererSinkCache { ...@@ -67,7 +67,7 @@ class MODULES_EXPORT AudioRendererSinkCache {
private: private:
friend class AudioRendererSinkCacheTest; friend class AudioRendererSinkCacheTest;
friend class CacheEntryFinder; friend class CacheEntryFinder;
friend class AudioRendererSinkCache::FrameObserver; friend class AudioRendererSinkCache::WindowObserver;
struct CacheEntry; struct CacheEntry;
using CacheContainer = std::vector<CacheEntry>; using CacheContainer = std::vector<CacheEntry>;
......
...@@ -178,7 +178,7 @@ void ModulesInitializer::InstallSupplements(LocalFrame& frame) const { ...@@ -178,7 +178,7 @@ void ModulesInitializer::InstallSupplements(LocalFrame& frame) const {
DCHECK(WebLocalFrameImpl::FromFrame(&frame)->Client()); DCHECK(WebLocalFrameImpl::FromFrame(&frame)->Client());
InspectorAccessibilityAgent::ProvideTo(&frame); InspectorAccessibilityAgent::ProvideTo(&frame);
ImageDownloaderImpl::ProvideTo(frame); ImageDownloaderImpl::ProvideTo(frame);
AudioRendererSinkCache::InstallFrameObserver(frame); AudioRendererSinkCache::InstallWindowObserver(*frame.DomWindow());
} }
MediaControls* ModulesInitializer::CreateMediaControls( MediaControls* ModulesInitializer::CreateMediaControls(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment