Commit 9bb5626f authored by Abigail Klein's avatar Abigail Klein Committed by Chromium LUCI CQ

[Live Caption] Create live caption UI after SODA has been installed.

This fixes a bug where the live caption UI appeared while SODA was
downloading, and showed an error message. This CL has the caption
controller observe the SODA installer and only creates the UI when the
installation has completed, or if SODA is already installed.

Bug: 1055150, 1160272
Change-Id: I32bd21344c88e6bcf1c79399e52ce994254f50ef
AX-Relnotes: N/A
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2595841
Commit-Queue: Abigail Klein <abigailbklein@google.com>
Reviewed-by: default avatarEvan Liu <evliu@google.com>
Cr-Commit-Position: refs/heads/master@{#839763}
parent 955cc7e0
...@@ -101,18 +101,39 @@ void CaptionController::OnLiveCaptionEnabledChanged() { ...@@ -101,18 +101,39 @@ void CaptionController::OnLiveCaptionEnabledChanged() {
enabled_ = enabled; enabled_ = enabled;
if (enabled_) { if (enabled_) {
// Only create the UI when SODA is downloaded--otherwise, wait for the SODA
// download to complete before creating the UI. This checks whether SODA is
// registered, which implies that it has already downloaded previously. It's
// possible for someone to enable Live Caption while SODA is downloading, in
// which case the UI will construct prematurely.
// TODO(crbug.com/1160272): Check whether SODA has downloaded without
// blocking the process.
if (speech::SODAInstaller::GetInstance()->IsSODARegistered()) {
UpdateUIEnabled();
} else {
// Register SODA component and download speech model. // Register SODA component and download speech model.
g_browser_process->local_state()->SetTime(prefs::kSodaScheduledDeletionTime, g_browser_process->local_state()->SetTime(
base::Time()); prefs::kSodaScheduledDeletionTime, base::Time());
// Observe the SODA installation and call UpdateUIEnabled when it
// completes.
speech::SODAInstaller::GetInstance()->AddObserver(this);
speech::SODAInstaller::GetInstance()->InstallSODA(profile_->GetPrefs()); speech::SODAInstaller::GetInstance()->InstallSODA(profile_->GetPrefs());
speech::SODAInstaller::GetInstance()->InstallLanguage(profile_->GetPrefs()); speech::SODAInstaller::GetInstance()->InstallLanguage(
profile_->GetPrefs());
}
} else { } else {
// Schedule SODA to be deleted in 30 days if the feature is not enabled // Schedule SODA to be deleted in 30 days if the feature is not enabled
// before then. // before then.
g_browser_process->local_state()->SetTime( g_browser_process->local_state()->SetTime(
prefs::kSodaScheduledDeletionTime, prefs::kSodaScheduledDeletionTime,
base::Time::Now() + base::TimeDelta::FromDays(kSodaCleanUpDelayInDays)); base::Time::Now() + base::TimeDelta::FromDays(kSodaCleanUpDelayInDays));
UpdateUIEnabled();
} }
}
void CaptionController::OnSODAInstalled() {
DCHECK(enabled_);
speech::SODAInstaller::GetInstance()->RemoveObserver(this);
UpdateUIEnabled(); UpdateUIEnabled();
} }
...@@ -128,6 +149,9 @@ bool CaptionController::IsLiveCaptionEnabled() { ...@@ -128,6 +149,9 @@ bool CaptionController::IsLiveCaptionEnabled() {
void CaptionController::UpdateUIEnabled() { void CaptionController::UpdateUIEnabled() {
if (enabled_) { if (enabled_) {
if (is_ui_constructed_)
return;
is_ui_constructed_ = true;
// Create captions UI in each browser view. // Create captions UI in each browser view.
for (Browser* browser : *BrowserList::GetInstance()) { for (Browser* browser : *BrowserList::GetInstance()) {
OnBrowserAdded(browser); OnBrowserAdded(browser);
...@@ -138,12 +162,16 @@ void CaptionController::UpdateUIEnabled() { ...@@ -138,12 +162,16 @@ void CaptionController::UpdateUIEnabled() {
// Observe caption style prefs. // Observe caption style prefs.
for (const char* const pref_name : kCaptionStylePrefsToObserve) { for (const char* const pref_name : kCaptionStylePrefsToObserve) {
DCHECK(!pref_change_registrar_->IsObserved(pref_name));
pref_change_registrar_->Add( pref_change_registrar_->Add(
pref_name, base::BindRepeating(&CaptionController::UpdateCaptionStyle, pref_name, base::BindRepeating(&CaptionController::UpdateCaptionStyle,
base::Unretained(this))); base::Unretained(this)));
} }
UpdateCaptionStyle(); UpdateCaptionStyle();
} else { } else {
if (!is_ui_constructed_)
return;
is_ui_constructed_ = false;
// Destroy caption bubble controllers. // Destroy caption bubble controllers.
caption_bubble_controllers_.clear(); caption_bubble_controllers_.clear();
...@@ -152,6 +180,7 @@ void CaptionController::UpdateUIEnabled() { ...@@ -152,6 +180,7 @@ void CaptionController::UpdateUIEnabled() {
// Remove prefs to observe. // Remove prefs to observe.
for (const char* const pref_name : kCaptionStylePrefsToObserve) { for (const char* const pref_name : kCaptionStylePrefsToObserve) {
DCHECK(pref_change_registrar_->IsObserved(pref_name));
pref_change_registrar_->Remove(pref_name); pref_change_registrar_->Remove(pref_name);
} }
} }
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <memory> #include <memory>
#include <unordered_map> #include <unordered_map>
#include "chrome/browser/accessibility/soda_installer.h"
#include "chrome/browser/ui/browser_list_observer.h" #include "chrome/browser/ui/browser_list_observer.h"
#include "chrome/common/caption.mojom.h" #include "chrome/common/caption.mojom.h"
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
...@@ -38,7 +39,9 @@ class CaptionBubbleController; ...@@ -38,7 +39,9 @@ class CaptionBubbleController;
// per profile and it lasts for the duration of the session. The caption // per profile and it lasts for the duration of the session. The caption
// controller owns the live caption UI, which are caption bubble controllers. // controller owns the live caption UI, which are caption bubble controllers.
// //
class CaptionController : public BrowserListObserver, public KeyedService { class CaptionController : public BrowserListObserver,
public KeyedService,
public speech::SODAInstaller::Observer {
public: public:
explicit CaptionController(Profile* profile); explicit CaptionController(Profile* profile);
~CaptionController() override; ~CaptionController() override;
...@@ -71,6 +74,11 @@ class CaptionController : public BrowserListObserver, public KeyedService { ...@@ -71,6 +74,11 @@ class CaptionController : public BrowserListObserver, public KeyedService {
void OnBrowserAdded(Browser* browser) override; void OnBrowserAdded(Browser* browser) override;
void OnBrowserRemoved(Browser* browser) override; void OnBrowserRemoved(Browser* browser) override;
// SODAInstaller::Observer:
void OnSODAInstalled() override;
void OnSODAProgress(int progress) override {}
void OnSODAError() override {}
void OnLiveCaptionEnabledChanged(); void OnLiveCaptionEnabledChanged();
void OnLiveCaptionLanguageChanged(); void OnLiveCaptionLanguageChanged();
bool IsLiveCaptionEnabled(); bool IsLiveCaptionEnabled();
...@@ -91,7 +99,13 @@ class CaptionController : public BrowserListObserver, public KeyedService { ...@@ -91,7 +99,13 @@ class CaptionController : public BrowserListObserver, public KeyedService {
base::Optional<ui::CaptionStyle> caption_style_; base::Optional<ui::CaptionStyle> caption_style_;
// Whether Live Caption is enabled.
bool enabled_ = false; bool enabled_ = false;
// Whether the UI has been created. The UI is created asynchronously from the
// feature being enabled--we wait for SODA to download first. This flag
// ensures that the UI is not constructed or deconstructed twice.
bool is_ui_constructed_ = false;
}; };
} // namespace captions } // namespace captions
......
...@@ -54,13 +54,22 @@ class CaptionControllerTest : public InProcessBrowserTest { ...@@ -54,13 +54,22 @@ class CaptionControllerTest : public InProcessBrowserTest {
// InProcessBrowserTest overrides: // InProcessBrowserTest overrides:
void SetUp() override { void SetUp() override {
scoped_feature_list_.InitAndEnableFeature(media::kLiveCaption); scoped_feature_list_.InitWithFeatures(
{media::kLiveCaption, media::kUseSodaForLiveCaption}, {});
InProcessBrowserTest::SetUp(); InProcessBrowserTest::SetUp();
} }
void SetLiveCaptionEnabled(bool enabled) { void SetLiveCaptionEnabled(bool enabled) {
browser()->profile()->GetPrefs()->SetBoolean(prefs::kLiveCaptionEnabled, browser()->profile()->GetPrefs()->SetBoolean(prefs::kLiveCaptionEnabled,
enabled); enabled);
if (enabled)
speech::SODAInstaller::GetInstance()->NotifySODAInstalledForTesting();
}
void SetLiveCaptionEnabledForProfile(bool enabled, Profile* profile) {
profile->GetPrefs()->SetBoolean(prefs::kLiveCaptionEnabled, enabled);
if (enabled)
speech::SODAInstaller::GetInstance()->NotifySODAInstalledForTesting();
} }
CaptionController* GetController() { CaptionController* GetController() {
...@@ -200,6 +209,17 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest, ...@@ -200,6 +209,17 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest,
EXPECT_EQ(0, NumBubbleControllers()); EXPECT_EQ(0, NumBubbleControllers());
} }
IN_PROC_BROWSER_TEST_F(CaptionControllerTest, OnSODAInstalled) {
EXPECT_EQ(0, NumBubbleControllers());
browser()->profile()->GetPrefs()->SetBoolean(prefs::kLiveCaptionEnabled,
true);
EXPECT_EQ(0, NumBubbleControllers());
// The UI is only created after SODA is installed.
speech::SODAInstaller::GetInstance()->NotifySODAInstalledForTesting();
EXPECT_EQ(1, NumBubbleControllers());
}
IN_PROC_BROWSER_TEST_F(CaptionControllerTest, OnBrowserAdded) { IN_PROC_BROWSER_TEST_F(CaptionControllerTest, OnBrowserAdded) {
EXPECT_EQ(0, NumBubbleControllers()); EXPECT_EQ(0, NumBubbleControllers());
...@@ -492,7 +512,7 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest, ...@@ -492,7 +512,7 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest,
EXPECT_EQ(0, NumBubbleControllersForProfile(profile2)); EXPECT_EQ(0, NumBubbleControllersForProfile(profile2));
// Enable live caption on profile2. // Enable live caption on profile2.
profile2->GetPrefs()->SetBoolean(prefs::kLiveCaptionEnabled, true); SetLiveCaptionEnabledForProfile(true, profile2);
EXPECT_EQ(1, NumBubbleControllersForProfile(profile1)); EXPECT_EQ(1, NumBubbleControllersForProfile(profile1));
EXPECT_EQ(1, NumBubbleControllersForProfile(profile2)); EXPECT_EQ(1, NumBubbleControllersForProfile(profile2));
...@@ -502,7 +522,7 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest, ...@@ -502,7 +522,7 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest,
EXPECT_EQ(1, NumBubbleControllersForProfile(profile2)); EXPECT_EQ(1, NumBubbleControllersForProfile(profile2));
// Disable live caption on profile2. // Disable live caption on profile2.
profile2->GetPrefs()->SetBoolean(prefs::kLiveCaptionEnabled, false); SetLiveCaptionEnabledForProfile(false, profile2);
EXPECT_EQ(0, NumBubbleControllersForProfile(profile1)); EXPECT_EQ(0, NumBubbleControllersForProfile(profile1));
EXPECT_EQ(0, NumBubbleControllersForProfile(profile2)); EXPECT_EQ(0, NumBubbleControllersForProfile(profile2));
} }
...@@ -513,7 +533,7 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest, OnBrowserAdded_MultipleProfiles) { ...@@ -513,7 +533,7 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest, OnBrowserAdded_MultipleProfiles) {
// Enable live caption on both profiles. // Enable live caption on both profiles.
SetLiveCaptionEnabled(true); SetLiveCaptionEnabled(true);
profile2->GetPrefs()->SetBoolean(prefs::kLiveCaptionEnabled, true); SetLiveCaptionEnabledForProfile(true, profile2);
// Add a new browser to profile1. Test that there are caption bubble // Add a new browser to profile1. Test that there are caption bubble
// controllers on all of the existing browsers. // controllers on all of the existing browsers.
...@@ -557,7 +577,7 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest, ...@@ -557,7 +577,7 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest,
// Enable live caption on both profiles. // Enable live caption on both profiles.
SetLiveCaptionEnabled(true); SetLiveCaptionEnabled(true);
profile2->GetPrefs()->SetBoolean(prefs::kLiveCaptionEnabled, true); SetLiveCaptionEnabledForProfile(true, profile2);
EXPECT_EQ(1, NumBubbleControllersForProfile(profile1)); EXPECT_EQ(1, NumBubbleControllersForProfile(profile1));
EXPECT_EQ(1, NumBubbleControllersForProfile(profile2)); EXPECT_EQ(1, NumBubbleControllersForProfile(profile2));
...@@ -596,7 +616,7 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest, ...@@ -596,7 +616,7 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest,
// Enable live caption on both profiles. // Enable live caption on both profiles.
SetLiveCaptionEnabled(true); SetLiveCaptionEnabled(true);
profile2->GetPrefs()->SetBoolean(prefs::kLiveCaptionEnabled, true); SetLiveCaptionEnabledForProfile(true, profile2);
// Dispatch transcription routes the transcription to the right browser on the // Dispatch transcription routes the transcription to the right browser on the
// right profile. // right profile.
...@@ -656,7 +676,7 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest, OnError_MultipleProfiles) { ...@@ -656,7 +676,7 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest, OnError_MultipleProfiles) {
// Enable live caption on both profiles. // Enable live caption on both profiles.
SetLiveCaptionEnabled(true); SetLiveCaptionEnabled(true);
profile2->GetPrefs()->SetBoolean(prefs::kLiveCaptionEnabled, true); SetLiveCaptionEnabledForProfile(true, profile2);
// OnError routes to the right browser on the right profile. // OnError routes to the right browser on the right profile.
OnErrorOnBrowserForProfile(browser1, profile1); OnErrorOnBrowserForProfile(browser1, profile1);
...@@ -689,6 +709,6 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest, OnError_MultipleProfiles) { ...@@ -689,6 +709,6 @@ IN_PROC_BROWSER_TEST_F(CaptionControllerTest, OnError_MultipleProfiles) {
#endif #endif
} }
#endif // !defined (OS_CHROMEOS) #endif // !BUILDFLAG(IS_CHROMEOS_ASH)
} // namespace captions } // namespace captions
...@@ -33,4 +33,9 @@ void SODAInstaller::NotifyOnSODAProgress(int percent) { ...@@ -33,4 +33,9 @@ void SODAInstaller::NotifyOnSODAProgress(int percent) {
observer.OnSODAProgress(percent); observer.OnSODAProgress(percent);
} }
void SODAInstaller::NotifySODAInstalledForTesting() {
if (!IsSODARegistered())
NotifyOnSODAInstalled();
}
} // namespace speech } // namespace speech
...@@ -51,12 +51,18 @@ class SODAInstaller { ...@@ -51,12 +51,18 @@ class SODAInstaller {
// should be. // should be.
virtual void InstallLanguage(PrefService* prefs) = 0; virtual void InstallLanguage(PrefService* prefs) = 0;
// Returns whether or not SODA is already registered on this device.
virtual bool IsSODARegistered() = 0;
// Adds an observer to the observer list. // Adds an observer to the observer list.
void AddObserver(Observer* observer); void AddObserver(Observer* observer);
// Removes an observer from the observer list. // Removes an observer from the observer list.
void RemoveObserver(Observer* observer); void RemoveObserver(Observer* observer);
void NotifySODAInstalledForTesting();
protected:
// Notifies the observers that the SODA installation has completed. // Notifies the observers that the SODA installation has completed.
void NotifyOnSODAInstalled(); void NotifyOnSODAInstalled();
...@@ -67,7 +73,6 @@ class SODAInstaller { ...@@ -67,7 +73,6 @@ class SODAInstaller {
// Progress is the download percentage out of 100. // Progress is the download percentage out of 100.
void NotifyOnSODAProgress(int progress); void NotifyOnSODAProgress(int progress);
protected:
base::ObserverList<Observer> observers_; base::ObserverList<Observer> observers_;
}; };
......
...@@ -6,9 +6,11 @@ ...@@ -6,9 +6,11 @@
#include <map> #include <map>
#include <string> #include <string>
#include <vector>
#include "base/bind.h" #include "base/bind.h"
#include "base/check_op.h" #include "base/check_op.h"
#include "base/feature_list.h"
#include "base/no_destructor.h" #include "base/no_destructor.h"
#include "base/numerics/ranges.h" #include "base/numerics/ranges.h"
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
...@@ -18,6 +20,7 @@ ...@@ -18,6 +20,7 @@
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/update_client/crx_update_item.h" #include "components/update_client/crx_update_item.h"
#include "media/base/media_switches.h"
namespace { namespace {
...@@ -83,6 +86,26 @@ void SODAInstallerImpl::InstallLanguage(PrefService* prefs) { ...@@ -83,6 +86,26 @@ void SODAInstallerImpl::InstallLanguage(PrefService* prefs) {
} }
} }
bool SODAInstallerImpl::IsSODARegistered() {
if (!base::FeatureList::IsEnabled(media::kUseSodaForLiveCaption))
return true;
std::vector<std::string> component_ids =
g_browser_process->component_updater()->GetComponentIDs();
bool has_soda = false;
bool has_language_pack = false;
for (std::string id : component_ids) {
if (id == component_updater::SODAComponentInstallerPolicy::GetExtensionId())
has_soda = true;
if (id == component_updater::SodaEnUsComponentInstallerPolicy::
GetExtensionId() ||
id == component_updater::SodaJaJpComponentInstallerPolicy::
GetExtensionId()) {
has_language_pack = true;
}
}
return has_soda && has_language_pack;
}
void SODAInstallerImpl::OnEvent(Events event, const std::string& id) { void SODAInstallerImpl::OnEvent(Events event, const std::string& id) {
if (id != component_updater::SODAComponentInstallerPolicy::GetExtensionId() && if (id != component_updater::SODAComponentInstallerPolicy::GetExtensionId() &&
id != component_updater::SodaEnUsComponentInstallerPolicy:: id != component_updater::SodaEnUsComponentInstallerPolicy::
......
...@@ -33,6 +33,7 @@ class SODAInstallerImpl : public SODAInstaller, ...@@ -33,6 +33,7 @@ class SODAInstallerImpl : public SODAInstaller,
// SODAInstaller: // SODAInstaller:
void InstallSODA(PrefService* prefs) override; void InstallSODA(PrefService* prefs) override;
void InstallLanguage(PrefService* prefs) override; void InstallLanguage(PrefService* prefs) override;
bool IsSODARegistered() override;
private: private:
// component_updater::ServiceObserver: // component_updater::ServiceObserver:
......
...@@ -45,6 +45,11 @@ void SODAInstallerImplChromeOS::InstallLanguage(PrefService* prefs) { ...@@ -45,6 +45,11 @@ void SODAInstallerImplChromeOS::InstallLanguage(PrefService* prefs) {
// TODO(crbug.com/1111002): Install SODA language. // TODO(crbug.com/1111002): Install SODA language.
} }
bool SODAInstallerImplChromeOS::IsSODARegistered() {
// TODO(crbug.com/1111002): Return whether SODA is registered.
return !base::FeatureList::IsEnabled(media::kUseSodaForLiveCaption);
}
void SODAInstallerImplChromeOS::OnSODAInstalled( void SODAInstallerImplChromeOS::OnSODAInstalled(
const chromeos::DlcserviceClient::InstallResult& install_result) { const chromeos::DlcserviceClient::InstallResult& install_result) {
if (install_result.error == dlcservice::kErrorNone) { if (install_result.error == dlcservice::kErrorNone) {
......
...@@ -25,6 +25,7 @@ class SODAInstallerImplChromeOS : public SODAInstaller { ...@@ -25,6 +25,7 @@ class SODAInstallerImplChromeOS : public SODAInstaller {
// SODAInstaller: // SODAInstaller:
void InstallSODA(PrefService* prefs) override; void InstallSODA(PrefService* prefs) override;
void InstallLanguage(PrefService* prefs) override; void InstallLanguage(PrefService* prefs) override;
bool IsSODARegistered() override;
private: private:
// This function is the InstallCallback for DlcserviceClient::Install(). // This function is the InstallCallback for DlcserviceClient::Install().
......
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