This change to the ownership model for the delegate passed into `SetEventDelegate` requires us to separate the controller from the delegate, and have them share a weak relationship. Chromium changes: https://chromium.googlesource.com/chromium/src/+/50d43c7bcf04f5fac7cdc7ad26732f3d94b5bd43 commit 50d43c7bcf04f5fac7cdc7ad26732f3d94b5bd43 Author: Di Wu <diwux@google.com> Date: Sun Jul 27 20:29:49 2025 -0700 Refactor TtsUtterance to always own its event delegate This change refactors the ownership model for UtteranceEventDelegate to establish a single, clear model where TtsUtterance always takes ownership of its delegate via std::unique_ptr. This simplifies the code, addresses code review feedback, and fixes underlying memory management issues that caused test failures. Problem: The previous memory management for UtteranceEventDelegate was fragile. Some delegates used a "delete this" pattern, which is error-prone and led to memory leaks in browser tests. An initial refactoring moved to a std::unique_ptr model but introduced a NonOwnedUtteranceEventDelegate wrapper to handle cases where the delegate's lifetime was managed externally (e.g., TtsSpeakFunction, SettingsWithTtsPreviewHandler). Code reviewers pointed out that this wrapper complicated the ownership model and that a single, consistent ownership pattern would be preferable. Solution: This commit fully adopts the single-ownership model and removes the NonOwnedUtteranceEventDelegate wrapper. 1. TtsUtterance always owns its delegate: TtsUtterance::SetEventDelegate now exclusively takes a std::unique_ptr<UtteranceEventDelegate>, ensuring the delegate is automatically destroyed with the utterance. 2. Refactored externally-managed delegates: The two cases that previously required a non-owned wrapper have been refactored to use dedicated, owned delegate classes: 2.1 TtsSpeakFunction: Now uses a new, private TtsExtensionEventHandler class that implements UtteranceEventDelegate. This handler is owned by the TtsUtterance and holds the `extension_id` to dispatch events. This removes the need for a reference back to the `TtsSpeakFunction`, simplifying lifetime management. 2.2 SettingsWithTtsPreviewHandler: Now uses a new TtsPreviewEventDelegate class. This handler is owned by the TtsUtterance and holds a base::WeakPtr to the SettingsWithTtsPreviewHandler to safely make callbacks. 3. Simplified delegate cleanup: With the removal of the non-owned wrapper, the GetType() virtual method on UtteranceEventDelegate and the corresponding logic in TtsControllerImpl are no longer needed and have been removed. This refactoring makes the TTS delegate ownership model clear, consistent, and safe, resolving memory leaks and directly addressing code review feedback for a simpler design. Bug: b:281717553, b:251732518, 431531726, 432151731 Test: Run and symbolize the affected two tests with asan and lsan enabled. Then run with MiraclePtr check on the SelectToSpeakTest.FullscreenMagnifierFollowsTextBoundsWhenPrefOn test. Change-Id: Ia7524c0a859ee2f7c1f208412215c6e6b1b4e512 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6733896 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Commit-Queue: Di Wu <diwux@google.com> Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Cr-Commit-Position: refs/heads/main@{#1492580}
158 lines
4.5 KiB
C++
158 lines
4.5 KiB
C++
// Copyright (c) 2023 The Brave Authors. All rights reserved.
|
|
// This Source Code Form is subject to the terms of the Mozilla Public
|
|
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
|
|
// You can obtain one at https://mozilla.org/MPL/2.0/.
|
|
|
|
#ifndef BRAVE_COMPONENTS_SPEEDREADER_TTS_PLAYER_H_
|
|
#define BRAVE_COMPONENTS_SPEEDREADER_TTS_PLAYER_H_
|
|
|
|
#include <memory>
|
|
#include <optional>
|
|
#include <string>
|
|
#include <utility>
|
|
|
|
#include "base/functional/callback.h"
|
|
#include "base/memory/raw_ptr.h"
|
|
#include "base/memory/singleton.h"
|
|
#include "base/memory/weak_ptr.h"
|
|
#include "base/observer_list.h"
|
|
#include "base/types/pass_key.h"
|
|
#include "base/values.h"
|
|
#include "content/public/browser/tts_utterance.h"
|
|
#include "content/public/browser/web_contents_observer.h"
|
|
|
|
namespace content {
|
|
class WebContents;
|
|
}
|
|
|
|
namespace speedreader {
|
|
|
|
// Browser-wide singleton that provides Text-to-speech functionality for
|
|
// speedreader.
|
|
class TtsPlayer {
|
|
public:
|
|
// Delegate for getting a text content of the WebContents for playing.
|
|
// TtsPlayer owns this in unique_ptr.
|
|
struct Delegate {
|
|
virtual ~Delegate() = default;
|
|
|
|
virtual void RequestReadingContent(
|
|
content::WebContents* web_contents,
|
|
base::OnceCallback<void(base::Value content)> result_cb) = 0;
|
|
};
|
|
|
|
class Observer : public base::CheckedObserver {
|
|
public:
|
|
virtual void OnReadingStart(content::WebContents* web_contents) {}
|
|
virtual void OnReadingStop(content::WebContents* web_contents) {}
|
|
virtual void OnReadingProgress(content::WebContents* web_contents,
|
|
int tts_order,
|
|
int char_index,
|
|
int length) {}
|
|
|
|
protected:
|
|
~Observer() override = default;
|
|
};
|
|
|
|
// Provides tts control fucntions for specified WebContents (provided by
|
|
// TtsPlayer::GetControllerFor). Controller is a part of TtsPlayer and has
|
|
// same lifetime.
|
|
class Controller : public content::WebContentsObserver {
|
|
public:
|
|
bool IsPlaying() const;
|
|
bool IsPlayingRequestedWebContents(
|
|
std::optional<int> paragraph_index = std::nullopt) const;
|
|
|
|
void Play(std::optional<int> paragraph_index = std::nullopt);
|
|
void Pause();
|
|
void Resume();
|
|
void Stop();
|
|
void Forward();
|
|
void Rewind();
|
|
|
|
class Delegate;
|
|
|
|
void OnTtsEvent(base::PassKey<Delegate>,
|
|
content::TtsUtterance* utterance,
|
|
content::TtsEventType event_type,
|
|
int char_index,
|
|
int length,
|
|
const std::string& error_message);
|
|
|
|
private:
|
|
explicit Controller(TtsPlayer* owner);
|
|
~Controller() override;
|
|
|
|
friend class TtsPlayer;
|
|
|
|
void SetRequestWebContents(content::WebContents* web_contents);
|
|
|
|
void Resume(bool recreate_utterance);
|
|
|
|
bool HasNextParagraph();
|
|
std::u16string GetParagraphToRead();
|
|
|
|
// content::WebContentsObserver:
|
|
void DidStartNavigation(content::NavigationHandle* handle) override;
|
|
void WebContentsDestroyed() override;
|
|
|
|
void OnContentReady(content::WebContents* web_contents,
|
|
std::optional<int> paragraph_index,
|
|
base::Value content);
|
|
|
|
raw_ptr<TtsPlayer> owner_ = nullptr;
|
|
|
|
raw_ptr<content::WebContents, DanglingUntriaged> playing_web_contents_ =
|
|
nullptr;
|
|
raw_ptr<content::WebContents, DanglingUntriaged> request_web_contents_ =
|
|
nullptr;
|
|
|
|
int paragraph_index_ = -1;
|
|
int reading_start_position_ = 0;
|
|
int reading_position_ = 0;
|
|
base::Value reading_content_;
|
|
|
|
double current_speed_ = 1.0;
|
|
std::string current_voice_;
|
|
|
|
bool continue_next_paragraph_ = false;
|
|
base::WeakPtrFactory<Controller> weak_factory_{this};
|
|
};
|
|
|
|
~TtsPlayer();
|
|
|
|
TtsPlayer(const TtsPlayer&) = delete;
|
|
TtsPlayer(TtsPlayer&&) = delete;
|
|
TtsPlayer& operator=(const TtsPlayer&) = delete;
|
|
|
|
static TtsPlayer* GetInstance();
|
|
|
|
void set_delegate(std::unique_ptr<Delegate> delegate) {
|
|
delegate_ = std::move(delegate);
|
|
}
|
|
|
|
void AddObserver(Observer* observer);
|
|
void RemoveObserver(Observer* observer);
|
|
|
|
double GetSpeed() const;
|
|
const std::string& GetVoice() const;
|
|
|
|
void SetSpeed(double speed);
|
|
void SetVoice(const std::string& voice);
|
|
|
|
Controller& GetControllerFor(content::WebContents* web_contents);
|
|
|
|
private:
|
|
friend struct base::DefaultSingletonTraits<TtsPlayer>;
|
|
|
|
TtsPlayer();
|
|
|
|
std::unique_ptr<Delegate> delegate_;
|
|
Controller controller_{this};
|
|
base::ObserverList<Observer> observers_;
|
|
};
|
|
|
|
} // namespace speedreader
|
|
|
|
#endif // BRAVE_COMPONENTS_SPEEDREADER_TTS_PLAYER_H_
|