diff options
Diffstat (limited to 'chromium/third_party/blink/renderer')
10 files changed, 290 insertions, 129 deletions
diff --git a/chromium/third_party/blink/renderer/core/frame/local_frame.cc b/chromium/third_party/blink/renderer/core/frame/local_frame.cc index 74d1e79e332..f4c5046411e 100644 --- a/chromium/third_party/blink/renderer/core/frame/local_frame.cc +++ b/chromium/third_party/blink/renderer/core/frame/local_frame.cc @@ -752,22 +752,25 @@ const SecurityContext* LocalFrame::GetSecurityContext() const { return DomWindow() ? &DomWindow()->GetSecurityContext() : nullptr; } -void LocalFrame::PrintNavigationErrorMessage(const Frame& target_frame, - const char* reason) { +// Provides a string description of the Frame as either its URL or origin if +// remote. +static String FrameDescription(const Frame& frame) { // URLs aren't available for RemoteFrames, so the error message uses their // origin instead. - auto* target_local_frame = DynamicTo<LocalFrame>(&target_frame); - String target_frame_description = - target_local_frame - ? "with URL '" + - target_local_frame->GetDocument()->Url().GetString() + "'" - : "with origin '" + - target_frame.GetSecurityContext() - ->GetSecurityOrigin() - ->ToString() + - "'"; + const LocalFrame* local_frame = DynamicTo<LocalFrame>(&frame); + return local_frame + ? "with URL '" + local_frame->GetDocument()->Url().GetString() + "'" + : "with origin '" + + frame.GetSecurityContext() + ->GetSecurityOrigin() + ->ToString() + + "'"; +} + +void LocalFrame::PrintNavigationErrorMessage(const Frame& target_frame, + const String& reason) { String message = "Unsafe attempt to initiate navigation for frame " + - target_frame_description + " from frame with URL '" + + FrameDescription(target_frame)+ " from frame with URL '" + GetDocument()->Url().GetString() + "'. " + reason + "\n"; DomWindow()->PrintErrorMessage(message); @@ -1635,100 +1638,152 @@ static bool CanAccessAncestor(const SecurityOrigin& active_security_origin, return false; } -bool LocalFrame::CanNavigate(const Frame& target_frame, - const KURL& destination_url) { - if (&target_frame == this) +// `initiating_frame` - The frame that CanNavigate was initially requested for. +// `source_frame` - The frame that is currently being tested to see if it can +// navigate `target_frame`. +// `target_frame` - The frame to be navigated. +// `destination_url` - The URL to navigate to on `target_frame`. +static bool CanNavigateHelper(LocalFrame& initiating_frame, + const Frame& source_frame, + const Frame& target_frame, + const KURL& destination_url) { + // The only time the helper is called with a different `initiating_frame` from + // its `source_frame` is to recursively check if ancestors can navigate the + // top frame. + DCHECK(&initiating_frame == &source_frame || + target_frame == initiating_frame.Tree().Top()); + + // Only report navigation blocking on the initial call to CanNavigateHelper, + // not the recursive calls. + bool should_report = &initiating_frame == &source_frame; + + if (&target_frame == &source_frame) return true; // Navigating window.opener cross origin, without user activation. See // https://crbug.com/813643. - if (Opener() == target_frame && !HasTransientUserActivation(this) && + if (should_report && source_frame.Opener() == target_frame && + !source_frame.HasTransientUserActivation() && !target_frame.GetSecurityContext()->GetSecurityOrigin()->CanAccess( SecurityOrigin::Create(destination_url).get())) { - UseCounter::Count(GetDocument(), + UseCounter::Count(initiating_frame.GetDocument(), WebFeature::kOpenerNavigationWithoutGesture); } if (destination_url.ProtocolIsJavaScript() && - !GetSecurityContext()->GetSecurityOrigin()->CanAccess( + !source_frame.GetSecurityContext()->GetSecurityOrigin()->CanAccess( target_frame.GetSecurityContext()->GetSecurityOrigin())) { - PrintNavigationErrorMessage( - target_frame, - "The frame attempting navigation must be same-origin with the target " - "if navigating to a javascript: url"); + if (should_report) { + initiating_frame.PrintNavigationErrorMessage( + target_frame, + "The frame attempting navigation must be same-origin with the target " + "if navigating to a javascript: url"); + } return false; } - if (GetSecurityContext()->IsSandboxed( + if (source_frame.GetSecurityContext()->IsSandboxed( network::mojom::blink::WebSandboxFlags::kNavigation)) { - if (!target_frame.Tree().IsDescendantOf(this) && + if (!target_frame.Tree().IsDescendantOf(&source_frame) && !target_frame.IsMainFrame()) { - PrintNavigationErrorMessage( - target_frame, - "The frame attempting navigation is sandboxed, and is therefore " - "disallowed from navigating its ancestors."); + if (should_report) { + initiating_frame.PrintNavigationErrorMessage( + target_frame, + "The frame attempting navigation is sandboxed, and is therefore " + "disallowed from navigating its ancestors."); + } return false; } // Sandboxed frames can also navigate popups, if the // 'allow-sandbox-escape-via-popup' flag is specified, or if // 'allow-popups' flag is specified, or if the - if (target_frame.IsMainFrame() && target_frame != Tree().Top() && - GetSecurityContext()->IsSandboxed( + if (target_frame.IsMainFrame() && + target_frame != source_frame.Tree().Top() && + source_frame.GetSecurityContext()->IsSandboxed( network::mojom::blink::WebSandboxFlags:: kPropagatesToAuxiliaryBrowsingContexts) && - (GetSecurityContext()->IsSandboxed( + (source_frame.GetSecurityContext()->IsSandboxed( network::mojom::blink::WebSandboxFlags::kPopups) || - target_frame.Opener() != this)) { - PrintNavigationErrorMessage( - target_frame, - "The frame attempting navigation is sandboxed and is trying " - "to navigate a popup, but is not the popup's opener and is not " - "set to propagate sandboxing to popups."); + target_frame.Opener() != &source_frame)) { + if (should_report) { + initiating_frame.PrintNavigationErrorMessage( + target_frame, + "The frame attempting navigation is sandboxed and is trying " + "to navigate a popup, but is not the popup's opener and is not " + "set to propagate sandboxing to popups."); + } return false; } - // Top navigation is forbidden unless opted-in. allow-top-navigation or - // allow-top-navigation-by-user-activation will also skips origin checks. - if (target_frame == Tree().Top()) { - if (GetSecurityContext()->IsSandboxed( + // Top navigation is forbidden in sandboxed frames unless opted-in, and only + // then if the ancestor chain allowed to navigate the top frame. + if (target_frame == source_frame.Tree().Top()) { + if (source_frame.GetSecurityContext()->IsSandboxed( network::mojom::blink::WebSandboxFlags::kTopNavigation) && - GetSecurityContext()->IsSandboxed( + source_frame.GetSecurityContext()->IsSandboxed( network::mojom::blink::WebSandboxFlags:: kTopNavigationByUserActivation)) { - PrintNavigationErrorMessage( - target_frame, - "The frame attempting navigation of the top-level window is " - "sandboxed, but the flag of 'allow-top-navigation' or " - "'allow-top-navigation-by-user-activation' is not set."); + if (should_report) { + initiating_frame.PrintNavigationErrorMessage( + target_frame, + "The frame attempting navigation of the top-level window is " + "sandboxed, but the flag of 'allow-top-navigation' or " + "'allow-top-navigation-by-user-activation' is not set."); + } return false; } - if (GetSecurityContext()->IsSandboxed( + if (source_frame.GetSecurityContext()->IsSandboxed( network::mojom::blink::WebSandboxFlags::kTopNavigation) && - !GetSecurityContext()->IsSandboxed( + !source_frame.GetSecurityContext()->IsSandboxed( network::mojom::blink::WebSandboxFlags:: kTopNavigationByUserActivation) && - !LocalFrame::HasTransientUserActivation(this)) { - // With only 'allow-top-navigation-by-user-activation' (but not - // 'allow-top-navigation'), top navigation requires a user gesture. - GetLocalFrameHostRemote().DidBlockNavigation( - destination_url, GetDocument()->Url(), - mojom::NavigationBlockedReason::kRedirectWithNoUserGestureSandbox); - PrintNavigationErrorMessage( - target_frame, - "The frame attempting navigation of the top-level window is " - "sandboxed with the 'allow-top-navigation-by-user-activation' " - "flag, but has no user activation (aka gesture). See " - "https://www.chromestatus.com/feature/5629582019395584."); + !source_frame.HasTransientUserActivation()) { + if (should_report) { + // With only 'allow-top-navigation-by-user-activation' (but not + // 'allow-top-navigation'), top navigation requires a user gesture. + initiating_frame.GetLocalFrameHostRemote().DidBlockNavigation( + destination_url, initiating_frame.GetDocument()->Url(), + mojom::NavigationBlockedReason:: + kRedirectWithNoUserGestureSandbox); + initiating_frame.PrintNavigationErrorMessage( + target_frame, + "The frame attempting navigation of the top-level window is " + "sandboxed with the 'allow-top-navigation-by-user-activation' " + "flag, but has no user activation (aka gesture). See " + "https://www.chromestatus.com/feature/5629582019395584."); + } return false; } + + // If the nearest non-sandboxed ancestor frame is not allowed to navigate, + // then this sandboxed frame can't either. This prevents a cross-origin + // frame from embedding a sandboxed iframe with kTopNavigate from + // navigating the top frame. See (crbug.com/1145553) + if (Frame* parent_frame = source_frame.Tree().Parent()) { + bool parent_can_navigate = CanNavigateHelper( + initiating_frame, *parent_frame, target_frame, destination_url); + if (!parent_can_navigate) { + if (should_report) { + String message = + "The frame attempting navigation of the top-level window is " + "sandboxed and is not allowed to navigate since its ancestor " + "frame " + + FrameDescription(*parent_frame) + + " is unable to navigate the top frame.\n"; + initiating_frame.PrintNavigationErrorMessage(target_frame, message); + } + return false; + } + } return true; } } - DCHECK(GetSecurityContext()->GetSecurityOrigin()); - const SecurityOrigin& origin = *GetSecurityContext()->GetSecurityOrigin(); + DCHECK(source_frame.GetSecurityContext()->GetSecurityOrigin()); + const SecurityOrigin& origin = + *source_frame.GetSecurityContext()->GetSecurityOrigin(); // This is the normal case. A document can navigate its decendant frames, // or, more generally, a document can navigate a frame if the document is @@ -1752,17 +1807,17 @@ bool LocalFrame::CanNavigate(const Frame& target_frame, // and/or "parent" relation). Requiring some sort of relation prevents a // document from navigating arbitrary, unrelated top-level frames. if (!target_frame.Tree().Parent()) { - if (target_frame == Opener()) + if (target_frame == source_frame.Opener()) return true; if (CanAccessAncestor(origin, target_frame.Opener())) return true; } - if (target_frame == Tree().Top()) { + if (target_frame == source_frame.Tree().Top()) { // A frame navigating its top may blocked if the document initiating // the navigation has never received a user gesture and the navigation // isn't same-origin with the target. - if (HasStickyUserActivation() || + if (source_frame.HasStickyUserActivation() || target_frame.GetSecurityContext()->GetSecurityOrigin()->CanAccess( SecurityOrigin::Create(destination_url).get())) { return true; @@ -1774,32 +1829,50 @@ bool LocalFrame::CanNavigate(const Frame& target_frame, String destination_domain = network_utils::GetDomainAndRegistry( destination_url.Host(), network_utils::kIncludePrivateRegistries); if (!target_domain.IsEmpty() && !destination_domain.IsEmpty() && - target_domain == destination_domain) { + target_domain == destination_domain && + target_frame.GetSecurityContext()->GetSecurityOrigin()->Protocol() == + destination_url.Protocol()) { return true; } - if (auto* settings_client = Client()->GetContentSettingsClient()) { - if (settings_client->AllowPopupsAndRedirects(false /* default_value*/)) - return true; + + // We skip this check for recursive calls on remote frames, in which case + // we're less permissive. + if (const LocalFrame* local_frame = DynamicTo<LocalFrame>(&source_frame)) { + if (auto* settings_client = + local_frame->Client()->GetContentSettingsClient()) { + if (settings_client->AllowPopupsAndRedirects(false /* default_value*/)) + return true; + } + } + + if (should_report) { + initiating_frame.PrintNavigationErrorMessage( + target_frame, + "The frame attempting navigation is targeting its top-level window, " + "but is neither same-origin with its target nor has it received a " + "user gesture. See " + "https://www.chromestatus.com/features/5851021045661696."); + initiating_frame.GetLocalFrameHostRemote().DidBlockNavigation( + destination_url, initiating_frame.GetDocument()->Url(), + mojom::NavigationBlockedReason::kRedirectWithNoUserGesture); } - PrintNavigationErrorMessage( - target_frame, - "The frame attempting navigation is targeting its top-level window, " - "but is neither same-origin with its target nor has it received a " - "user gesture. See " - "https://www.chromestatus.com/features/5851021045661696."); - GetLocalFrameHostRemote().DidBlockNavigation( - destination_url, GetDocument()->Url(), - mojom::NavigationBlockedReason::kRedirectWithNoUserGesture); } else { - PrintNavigationErrorMessage(target_frame, - "The frame attempting navigation is neither " - "same-origin with the target, " - "nor is it the target's parent or opener."); + if (should_report) { + initiating_frame.PrintNavigationErrorMessage( + target_frame, + "The frame attempting navigation is neither same-origin with the " + "target, nor is it the target's parent or opener."); + } } return false; } +bool LocalFrame::CanNavigate(const Frame& target_frame, + const KURL& destination_url) { + return CanNavigateHelper(*this, *this, target_frame, destination_url); +} + ContentCaptureManager* LocalFrame::GetContentCaptureManager() { DCHECK(Client()); if (!IsLocalRoot()) diff --git a/chromium/third_party/blink/renderer/core/frame/local_frame.h b/chromium/third_party/blink/renderer/core/frame/local_frame.h index 38153330251..ec4edbd6f4c 100644 --- a/chromium/third_party/blink/renderer/core/frame/local_frame.h +++ b/chromium/third_party/blink/renderer/core/frame/local_frame.h @@ -186,7 +186,7 @@ class CORE_EXPORT LocalFrame final void Navigate(FrameLoadRequest&, WebFrameLoadType) override; bool ShouldClose() override; const SecurityContext* GetSecurityContext() const override; - void PrintNavigationErrorMessage(const Frame&, const char* reason); + void PrintNavigationErrorMessage(const Frame&, const String& reason); void PrintNavigationWarning(const String&); bool DetachDocument() override; void CheckCompleted() override; diff --git a/chromium/third_party/blink/renderer/core/loader/long_task_detector.cc b/chromium/third_party/blink/renderer/core/loader/long_task_detector.cc index 7e1499b1ddd..3816779cfaf 100644 --- a/chromium/third_party/blink/renderer/core/loader/long_task_detector.cc +++ b/chromium/third_party/blink/renderer/core/loader/long_task_detector.cc @@ -24,6 +24,7 @@ LongTaskDetector::LongTaskDetector() = default; void LongTaskDetector::RegisterObserver(LongTaskObserver* observer) { DCHECK(IsMainThread()); DCHECK(observer); + DCHECK(!iterating_); if (observers_.insert(observer).is_new_entry && observers_.size() == 1) { // Number of observers just became non-zero. Thread::Current()->AddTaskTimeObserver(this); @@ -32,6 +33,10 @@ void LongTaskDetector::RegisterObserver(LongTaskObserver* observer) { void LongTaskDetector::UnregisterObserver(LongTaskObserver* observer) { DCHECK(IsMainThread()); + if (iterating_) { + observers_to_be_removed_.push_back(observer); + return; + } observers_.erase(observer); if (observers_.size() == 0) { Thread::Current()->RemoveTaskTimeObserver(this); @@ -43,13 +48,21 @@ void LongTaskDetector::DidProcessTask(base::TimeTicks start_time, if ((end_time - start_time) < LongTaskDetector::kLongTaskThreshold) return; + iterating_ = true; for (auto& observer : observers_) { observer->OnLongTaskDetected(start_time, end_time); } + iterating_ = false; + + for (const auto& observer : observers_to_be_removed_) { + UnregisterObserver(observer); + } + observers_to_be_removed_.clear(); } void LongTaskDetector::Trace(Visitor* visitor) const { visitor->Trace(observers_); + visitor->Trace(observers_to_be_removed_); } } // namespace blink diff --git a/chromium/third_party/blink/renderer/core/loader/long_task_detector.h b/chromium/third_party/blink/renderer/core/loader/long_task_detector.h index dc6f0dbab5c..5fd4bb8d2ab 100644 --- a/chromium/third_party/blink/renderer/core/loader/long_task_detector.h +++ b/chromium/third_party/blink/renderer/core/loader/long_task_detector.h @@ -49,6 +49,8 @@ class CORE_EXPORT LongTaskDetector final base::TimeTicks end_time) override; HeapHashSet<Member<LongTaskObserver>> observers_; + HeapVector<Member<LongTaskObserver>> observers_to_be_removed_; + bool iterating_ = false; DISALLOW_COPY_AND_ASSIGN(LongTaskDetector); }; diff --git a/chromium/third_party/blink/renderer/core/loader/long_task_detector_test.cc b/chromium/third_party/blink/renderer/core/loader/long_task_detector_test.cc index 3384fa8ebfb..403ba452362 100644 --- a/chromium/third_party/blink/renderer/core/loader/long_task_detector_test.cc +++ b/chromium/third_party/blink/renderer/core/loader/long_task_detector_test.cc @@ -27,9 +27,26 @@ class TestLongTaskObserver : last_long_task_start = start_time; last_long_task_end = end_time; } -}; // Anonymous namespace +}; + +class SelfUnregisteringObserver + : public GarbageCollected<SelfUnregisteringObserver>, + public LongTaskObserver { + public: + void OnLongTaskDetected(base::TimeTicks, base::TimeTicks) override { + called_ = true; + LongTaskDetector::Instance().UnregisterObserver(this); + } + bool IsCalled() const { return called_; } + + void Reset() { called_ = false; } + + private: + bool called_ = false; +}; } // namespace + class LongTaskDetectorTest : public testing::Test { public: // Public because it's executed on a task queue. @@ -126,4 +143,18 @@ TEST_F(LongTaskDetectorTest, RegisterSameObserverTwice) { long_task_end_when_registered); } +TEST_F(LongTaskDetectorTest, SelfUnregisteringObserver) { + auto* observer = MakeGarbageCollected<SelfUnregisteringObserver>(); + + LongTaskDetector::Instance().RegisterObserver(observer); + SimulateTask(LongTaskDetector::kLongTaskThreshold + + base::TimeDelta::FromMilliseconds(10)); + EXPECT_TRUE(observer->IsCalled()); + observer->Reset(); + + SimulateTask(LongTaskDetector::kLongTaskThreshold + + base::TimeDelta::FromMilliseconds(10)); + EXPECT_FALSE(observer->IsCalled()); +} + } // namespace blink diff --git a/chromium/third_party/blink/renderer/modules/webaudio/audio_node.cc b/chromium/third_party/blink/renderer/modules/webaudio/audio_node.cc index 0b014b67ff4..4264c8d1127 100644 --- a/chromium/third_party/blink/renderer/modules/webaudio/audio_node.cc +++ b/chromium/third_party/blink/renderer/modules/webaudio/audio_node.cc @@ -614,13 +614,13 @@ void AudioNode::Dispose() { BaseAudioContext::GraphAutoLocker locker(context()); Handler().Dispose(); - // Add the handler to the orphan list if the context is pulling on the audio - // graph. This keeps the handler alive until it can be deleted at a safe - // point (in pre/post handler task). If graph isn't being pulled, we can - // delete the handler now since nothing on the audio thread will be touching - // it. + // Add the handler to the orphan list. This keeps the handler alive until it + // can be deleted at a safe point (in pre/post handler task). If the graph is + // being processed, the handler must be added. If the context is suspended, + // the handler still needs to be added in case the context is resumed. DCHECK(context()); - if (context()->IsPullingAudioGraph()) { + if (context()->IsPullingAudioGraph() || + context()->ContextState() == BaseAudioContext::kSuspended) { context()->GetDeferredTaskHandler().AddRenderingOrphanHandler( std::move(handler_)); } diff --git a/chromium/third_party/blink/renderer/modules/webaudio/audio_worklet_processor.cc b/chromium/third_party/blink/renderer/modules/webaudio/audio_worklet_processor.cc index 5583b8fd885..e68b1c1b2f6 100644 --- a/chromium/third_party/blink/renderer/modules/webaudio/audio_worklet_processor.cc +++ b/chromium/third_party/blink/renderer/modules/webaudio/audio_worklet_processor.cc @@ -180,11 +180,14 @@ bool AudioWorkletProcessor::PortTopologyMatches( if (audio_port_2.IsEmpty()) return false; - // Two AudioPorts are supposed to have the same length because the number of - // inputs and outputs of AudioNode cannot change after construction. v8::Local<v8::Array> port_2_local = audio_port_2.NewLocal(isolate); DCHECK(port_2_local->IsArray()); - DCHECK_EQ(audio_port_1.size(), port_2_local->Length()); + + // Two audio ports may have a different number of inputs or outputs. See + // crbug.com/1202060 + if (audio_port_1.size() != port_2_local->Length()) { + return false; + } v8::TryCatch try_catch(isolate); diff --git a/chromium/third_party/blink/renderer/modules/xr/xr_layer.cc b/chromium/third_party/blink/renderer/modules/xr/xr_layer.cc index eaa8603c735..aa30a4cec88 100644 --- a/chromium/third_party/blink/renderer/modules/xr/xr_layer.cc +++ b/chromium/third_party/blink/renderer/modules/xr/xr_layer.cc @@ -21,7 +21,7 @@ const AtomicString& XRLayer::InterfaceName() const { void XRLayer::Trace(Visitor* visitor) const { visitor->Trace(session_); - ScriptWrappable::Trace(visitor); + EventTargetWithInlineData::Trace(visitor); } } // namespace blink diff --git a/chromium/third_party/blink/renderer/platform/audio/fft_frame.h b/chromium/third_party/blink/renderer/platform/audio/fft_frame.h index 149b94c52f7..5474597147c 100644 --- a/chromium/third_party/blink/renderer/platform/audio/fft_frame.h +++ b/chromium/third_party/blink/renderer/platform/audio/fft_frame.h @@ -41,7 +41,7 @@ #if defined(WTF_USE_WEBAUDIO_FFMPEG) struct RDFTContext; #elif defined(WTF_USE_WEBAUDIO_PFFFT) -#include "third_party/blink/renderer/platform/wtf/vector.h" +#include "third_party/blink/renderer/platform/wtf/hash_map.h" #include "third_party/pffft/src/pffft.h" #elif defined(OS_MAC) #include <Accelerate/Accelerate.h> @@ -113,8 +113,14 @@ class PLATFORM_EXPORT FFTFrame { const FFTFrame& frame2, double x); - unsigned fft_size_; - unsigned log2fft_size_; + unsigned fft_size_ = 0; + + // When using PFFFT, this slot is not irrelevant and not used because PFFFT + // supports sizes that aren't a power of 2. + // TODO(https://crbug.com/988121) Look into whether Mac vDSP really needs + // this. + unsigned log2fft_size_ = 0; + // These two arrays contain the transformed data. Instead of a single array // of complex numbers, we split the complex data into an array of the real // part and the imaginary part. @@ -188,10 +194,10 @@ class PLATFORM_EXPORT FFTFrame { PFFFT_Setup* setup_; }; - // Returns the vector that holds all of the possible FFTSetup objects. This + // Returns the HashMap that holds all of the possible FFTSetup objects. This // should be setup in the |Initialize()| method that is called when a context // is created. - static Vector<std::unique_ptr<FFTSetup>>& FFTSetups(); + static HashMap<unsigned, std::unique_ptr<FFTSetup>>& FFTSetups(); // Initialize an entry in FFTSetups for an FFT of order |fft_order|. This can // be called from any thread, but if a new FFTSetup needs to be allocated, diff --git a/chromium/third_party/blink/renderer/platform/audio/pffft/fft_frame_pffft.cc b/chromium/third_party/blink/renderer/platform/audio/pffft/fft_frame_pffft.cc index 620188fff08..3317e8ad054 100644 --- a/chromium/third_party/blink/renderer/platform/audio/pffft/fft_frame_pffft.cc +++ b/chromium/third_party/blink/renderer/platform/audio/pffft/fft_frame_pffft.cc @@ -10,6 +10,7 @@ #include "third_party/blink/renderer/platform/audio/hrtf_panner.h" #include "third_party/blink/renderer/platform/audio/vector_math.h" #include "third_party/blink/renderer/platform/wtf/math_extras.h" +#include "third_party/blink/renderer/platform/wtf/threading_primitives.h" #include "third_party/pffft/src/pffft.h" namespace blink { @@ -38,45 +39,81 @@ FFTFrame::FFTSetup::~FFTSetup() { pffft_destroy_setup(setup_); } -Vector<std::unique_ptr<FFTFrame::FFTSetup>>& FFTFrame::FFTSetups() { +HashMap<unsigned, std::unique_ptr<FFTFrame::FFTSetup>>& FFTFrame::FFTSetups() { // TODO(rtoy): Let this bake for a bit and then remove the assertions after // we're confident the first call is from the main thread. static bool first_call = true; + // A HashMap to hold all of the possible FFT setups we need. The setups are + // initialized lazily. The key is the fft size, and the value is the setup + // data. + typedef HashMap<unsigned, std::unique_ptr<FFTSetup>> FFTHashMap_t; + + DEFINE_THREAD_SAFE_STATIC_LOCAL(FFTHashMap_t, fft_setups, ()); + if (first_call) { + DEFINE_STATIC_LOCAL(Mutex, setup_lock, ()); + // Make sure we construct the fft_setups vector below on the main thread. // Once constructed, we can access it from any thread. DCHECK(IsMainThread()); first_call = false; - } - // A vector to hold all of the possible FFT setups we need. The setups are - // initialized lazily. - DEFINE_STATIC_LOCAL(Vector<std::unique_ptr<FFTSetup>>, fft_setups, - (kMaxFFTPow2Size)); + MutexLocker locker(setup_lock); + + // Initialize the hash map with all the possible keys (FFT sizes), with a + // value of nullptr because we want to initialize the setup data lazily. The + // set of valid FFT sizes for PFFFT are of the form 2^k*3^m*5*n where k >= + // 5, m >= 0, n >= 0. We only go up to a max size of 32768, because we need + // at least an FFT size of 32768 for the convolver node. + + // TODO(crbug.com/988121): Sync this with kMaxFFTPow2Size. + const int kMaxConvolverFFTSize = 32768; + + for (int n = 1; n <= kMaxConvolverFFTSize; n *= 5) { + for (int m = 1; m <= kMaxConvolverFFTSize / n; m *= 3) { + for (int k = 32; k <= kMaxConvolverFFTSize / (n * m); k *= 2) { + int size = k * m * n; + if (size <= kMaxConvolverFFTSize && !fft_setups.Contains(size)) { + fft_setups.insert(size, nullptr); + } + } + } + } + + // There should be 87 entries when we're done. + DCHECK_EQ(fft_setups.size(), 87u); + } return fft_setups; } -void FFTFrame::InitializeFFTSetupForSize(wtf_size_t log2fft_size) { +void FFTFrame::InitializeFFTSetupForSize(wtf_size_t fft_size) { auto& setup = FFTSetups(); - if (!setup[log2fft_size]) { + DCHECK(setup.Contains(fft_size)); + + if (setup.find(fft_size)->value == nullptr) { + DEFINE_STATIC_LOCAL(Mutex, setup_lock, ()); + // Make sure allocation of a new setup only occurs on the main thread so we // don't have a race condition with multiple threads trying to write to the // same element of the vector. DCHECK(IsMainThread()); - setup[log2fft_size] = std::make_unique<FFTSetup>(1 << log2fft_size); + auto fft_data = std::make_unique<FFTSetup>(fft_size); + MutexLocker locker(setup_lock); + setup.find(fft_size)->value = std::move(fft_data); } } -PFFFT_Setup* FFTFrame::FFTSetupForSize(wtf_size_t log2fft_size) { +PFFFT_Setup* FFTFrame::FFTSetupForSize(wtf_size_t fft_size) { auto& setup = FFTSetups(); - DCHECK(setup[log2fft_size]); + DCHECK(setup.Contains(fft_size)); + DCHECK(setup.find(fft_size)->value); - return setup[log2fft_size]->GetSetup(); + return setup.find(fft_size)->value->GetSetup(); } FFTFrame::FFTFrame(unsigned fft_size) @@ -86,12 +123,10 @@ FFTFrame::FFTFrame(unsigned fft_size) imag_data_(fft_size / 2), complex_data_(fft_size), pffft_work_(fft_size) { - // We only allow power of two. - DCHECK_EQ(1UL << log2fft_size_, fft_size_); // Initialize the PFFFT_Setup object here so that it will be ready when we // compute FFTs. - InitializeFFTSetupForSize(log2fft_size_); + InitializeFFTSetupForSize(fft_size); } // Creates a blank/empty frame (interpolate() must later be called). @@ -107,7 +142,7 @@ FFTFrame::FFTFrame(const FFTFrame& frame) pffft_work_(frame.fft_size_) { // Initialize the PFFFT_Setup object here wo that it will be ready when we // compute FFTs. - InitializeFFTSetupForSize(log2fft_size_); + InitializeFFTSetupForSize(fft_size_); // Copy/setup frame data. unsigned nbytes = sizeof(float) * (fft_size_ / 2); @@ -134,21 +169,19 @@ void FFTFrame::Initialize(float sample_rate) { // // TODO(rtoy): Try to come up with some way so that |Initialize()| doesn't // need to know about how the HRTF panner uses FFTs. - unsigned hrtf_order = static_cast<unsigned>( - log2(HRTFPanner::FftSizeForSampleRate(sample_rate))); + unsigned hrtf_fft_size = + static_cast<unsigned>(HRTFPanner::FftSizeForSampleRate(sample_rate)); - DCHECK_GT(hrtf_order, kMinFFTPow2Size); - DCHECK_LE(hrtf_order, kMaxFFTPow2Size); + DCHECK_GT(hrtf_fft_size, 1U << kMinFFTPow2Size); + DCHECK_LE(hrtf_fft_size, 1U << kMaxFFTPow2Size); - InitializeFFTSetupForSize(hrtf_order); - InitializeFFTSetupForSize(hrtf_order - 1); + InitializeFFTSetupForSize(hrtf_fft_size); + InitializeFFTSetupForSize(hrtf_fft_size / 2); } void FFTFrame::Cleanup() { - auto& setups = FFTSetups(); - - for (wtf_size_t k = 0; k < setups.size(); ++k) { - setups[k].reset(); + for (auto& setup : FFTSetups()) { + setup.value.reset(); } } @@ -158,7 +191,7 @@ FFTFrame::~FFTFrame() { void FFTFrame::DoFFT(const float* data) { DCHECK_EQ(pffft_work_.size(), fft_size_); - PFFFT_Setup* setup = FFTSetupForSize(log2fft_size_); + PFFFT_Setup* setup = FFTSetupForSize(fft_size_); DCHECK(setup); pffft_transform_ordered(setup, data, complex_data_.Data(), pffft_work_.Data(), @@ -195,7 +228,7 @@ void FFTFrame::DoInverseFFT(float* data) { fft_data[index + 1] = imag[k]; } - PFFFT_Setup* setup = FFTSetupForSize(log2fft_size_); + PFFFT_Setup* setup = FFTSetupForSize(fft_size_); DCHECK(setup); pffft_transform_ordered(setup, fft_data, data, pffft_work_.Data(), |