Commit 7ad1d28c authored by Chris Lu's avatar Chris Lu Committed by Commit Bot

Use ObserverList for TranslateInfobarDelegate

TranslateInfobarDelegate is currently only allows for one
observer. This change replaces its observer_ property
with a ObserverList so that more than one object can
be an observer.

Change-Id: I67c2a1ee2e2bee8ba532bc4b4adc1d2237dc1bd8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2153330Reviewed-by: default avataranthonyvd <anthonyvd@chromium.org>
Reviewed-by: default avatarMoe Ahmadi <mahmadi@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760567}
parent fefb674a
......@@ -39,7 +39,7 @@ std::unique_ptr<infobars::InfoBar> ChromeTranslateClient::CreateInfoBar(
TranslateCompactInfoBar::TranslateCompactInfoBar(
std::unique_ptr<translate::TranslateInfoBarDelegate> delegate)
: InfoBarAndroid(std::move(delegate)), action_flags_(FLAG_NONE) {
GetDelegate()->SetObserver(this);
GetDelegate()->AddObserver(this);
// Flip the translate bit if auto translate is enabled.
if (GetDelegate()->translate_step() == translate::TRANSLATE_STEP_TRANSLATING)
......@@ -47,7 +47,7 @@ TranslateCompactInfoBar::TranslateCompactInfoBar(
}
TranslateCompactInfoBar::~TranslateCompactInfoBar() {
GetDelegate()->SetObserver(nullptr);
GetDelegate()->RemoveObserver(this);
}
ScopedJavaLocalRef<jobject> TranslateCompactInfoBar::CreateRenderInfoBar(
......@@ -225,5 +225,5 @@ bool TranslateCompactInfoBar::IsDeclinedByUser() {
void TranslateCompactInfoBar::OnTranslateInfoBarDelegateDestroyed(
translate::TranslateInfoBarDelegate* delegate) {
DCHECK_EQ(GetDelegate(), delegate);
GetDelegate()->SetObserver(nullptr);
GetDelegate()->RemoveObserver(this);
}
......@@ -49,7 +49,8 @@ class MockTranslateInfoBarDelegate
MOCK_CONST_METHOD1(language_name_at, base::string16(size_t index));
MOCK_CONST_METHOD0(original_language_name, base::string16());
MOCK_CONST_METHOD0(ShouldAlwaysTranslate, bool());
MOCK_METHOD1(SetObserver, void(Observer* observer));
MOCK_METHOD1(AddObserver, void(Observer* observer));
MOCK_METHOD1(RemoveObserver, void(Observer* observer));
};
class MockTranslateInfoBarDelegateFactory {
......
......@@ -52,8 +52,9 @@ const base::Feature kTranslateCompactUI{"TranslateCompactUI",
const size_t TranslateInfoBarDelegate::kNoIndex = TranslateUIDelegate::kNoIndex;
TranslateInfoBarDelegate::~TranslateInfoBarDelegate() {
if (observer_)
observer_->OnTranslateInfoBarDelegateDestroyed(this);
for (auto& observer : observers_) {
observer.OnTranslateInfoBarDelegateDestroyed(this);
}
}
infobars::InfoBarDelegate::InfoBarIdentifier
......@@ -111,8 +112,10 @@ void TranslateInfoBarDelegate::Create(
}
// Try to reuse existing translate infobar delegate.
if (old_delegate && old_delegate->observer_) {
old_delegate->observer_->OnTranslateStepChanged(step, error_type);
if (old_delegate) {
for (auto& observer : old_delegate->observers_) {
observer.OnTranslateStepChanged(step, error_type);
}
return;
}
......@@ -140,10 +143,6 @@ base::string16 TranslateInfoBarDelegate::language_name_at(size_t index) const {
return ui_delegate_.GetLanguageNameAt(index);
}
void TranslateInfoBarDelegate::SetObserver(Observer* observer) {
observer_ = observer;
}
base::string16 TranslateInfoBarDelegate::original_language_name() const {
return language_name_at(ui_delegate_.GetOriginalLanguageIndex());
}
......@@ -435,6 +434,14 @@ TranslateDriver* TranslateInfoBarDelegate::GetTranslateDriver() {
return translate_manager_->translate_client()->GetTranslateDriver();
}
void TranslateInfoBarDelegate::AddObserver(Observer* observer) {
observers_.AddObserver(observer);
}
void TranslateInfoBarDelegate::RemoveObserver(Observer* observer) {
observers_.RemoveObserver(observer);
}
TranslateInfoBarDelegate::TranslateInfoBarDelegate(
const base::WeakPtr<TranslateManager>& translate_manager,
bool is_off_the_record,
......@@ -450,8 +457,7 @@ TranslateInfoBarDelegate::TranslateInfoBarDelegate(
translate_manager_(translate_manager),
error_type_(error_type),
prefs_(translate_manager->translate_client()->GetTranslatePrefs()),
triggered_from_menu_(triggered_from_menu),
observer_(nullptr) {
triggered_from_menu_(triggered_from_menu) {
DCHECK_NE((step_ == translate::TRANSLATE_STEP_TRANSLATE_ERROR),
(error_type_ == TranslateErrors::NONE));
DCHECK(translate_manager_);
......@@ -462,9 +468,16 @@ int TranslateInfoBarDelegate::GetIconId() const {
}
void TranslateInfoBarDelegate::InfoBarDismissed() {
bool declined = observer_
? observer_->IsDeclinedByUser()
: (step_ == translate::TRANSLATE_STEP_BEFORE_TRANSLATE);
bool declined = false;
bool has_observer = false;
for (auto& observer : observers_) {
has_observer = true;
if (observer.IsDeclinedByUser())
declined = true;
}
if (!has_observer)
declined = step_ == translate::TRANSLATE_STEP_BEFORE_TRANSLATE;
if (declined) {
// The user closed the infobar without clicking the translate button.
......
......@@ -16,6 +16,8 @@
#include "base/logging.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h"
#include "base/observer_list_types.h"
#include "build/build_config.h"
#include "components/infobars/core/infobar_delegate.h"
#include "components/translate/core/browser/translate_prefs.h"
......@@ -43,7 +45,7 @@ class TranslateManager;
class TranslateInfoBarDelegate : public infobars::InfoBarDelegate {
public:
// An observer to handle different translate steps' UI changes.
class Observer {
class Observer : public base::CheckedObserver {
public:
// Handles UI changes on the translate step given.
virtual void OnTranslateStepChanged(translate::TranslateStep step,
......@@ -53,9 +55,6 @@ class TranslateInfoBarDelegate : public infobars::InfoBarDelegate {
// Called when the TranslateInfoBarDelegate instance is destroyed.
virtual void OnTranslateInfoBarDelegateDestroyed(
TranslateInfoBarDelegate* delegate) = 0;
protected:
virtual ~Observer() {}
};
static const size_t kNoIndex;
......@@ -216,8 +215,11 @@ class TranslateInfoBarDelegate : public infobars::InfoBarDelegate {
// May return NULL if the driver has been destroyed.
TranslateDriver* GetTranslateDriver();
// Set a observer.
virtual void SetObserver(Observer* observer);
// Add an observer.
virtual void AddObserver(Observer* observer);
// Remove an observer.
virtual void RemoveObserver(Observer* observer);
// InfoBarDelegate:
infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override;
......@@ -255,9 +257,9 @@ class TranslateInfoBarDelegate : public infobars::InfoBarDelegate {
// (due to language detection, preferences...)
bool triggered_from_menu_;
// A observer to handle front-end changes on different steps.
// Observers to handle front-end changes on different steps.
// It's only used when we try to reuse the existing UI.
Observer* observer_;
base::ObserverList<Observer> observers_;
DISALLOW_COPY_AND_ASSIGN(TranslateInfoBarDelegate);
};
......
......@@ -14,13 +14,13 @@ TranslateInfobarDelegateObserverBridge::TranslateInfobarDelegateObserverBridge(
translate::TranslateInfoBarDelegate* translate_infobar_delegate,
id<TranslateInfobarDelegateObserving> owner)
: translate_infobar_delegate_(translate_infobar_delegate), owner_(owner) {
translate_infobar_delegate_->SetObserver(this);
translate_infobar_delegate_->AddObserver(this);
}
TranslateInfobarDelegateObserverBridge::
~TranslateInfobarDelegateObserverBridge() {
if (translate_infobar_delegate_) {
translate_infobar_delegate_->SetObserver(nullptr);
translate_infobar_delegate_->RemoveObserver(this);
}
}
......@@ -41,6 +41,6 @@ void TranslateInfobarDelegateObserverBridge::
OnTranslateInfoBarDelegateDestroyed(
translate::TranslateInfoBarDelegate* delegate) {
DCHECK_EQ(translate_infobar_delegate_, delegate);
translate_infobar_delegate_->SetObserver(nullptr);
translate_infobar_delegate_->RemoveObserver(this);
translate_infobar_delegate_ = nullptr;
}
......@@ -56,7 +56,7 @@ class TranslateInfobarDelegateObserverBridgeTest : public PlatformTest {
: delegate_factory_("fr", "en"),
observer_([[TestTranslateInfoBarDelegateObserver alloc] init]) {
// Make sure the observer bridge sets up itself as an observer.
EXPECT_CALL(*GetDelegate(), SetObserver(_)).Times(1);
EXPECT_CALL(*GetDelegate(), AddObserver(_));
observer_bridge_ = std::make_unique<TranslateInfobarDelegateObserverBridge>(
delegate_factory_.GetMockTranslateInfoBarDelegate(), observer_);
......@@ -64,7 +64,7 @@ class TranslateInfobarDelegateObserverBridgeTest : public PlatformTest {
~TranslateInfobarDelegateObserverBridgeTest() override {
// Make sure the observer bridge removes itself as an observer.
EXPECT_CALL(*GetDelegate(), SetObserver(nullptr)).Times(1);
EXPECT_CALL(*GetDelegate(), RemoveObserver(_));
}
MockTranslateInfoBarDelegate* GetDelegate() {
......
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