Replace OnPageTranslate Observer by a responder of delegate.

In order to be notified that page is translated, we simply implement CompactInfobar as observer of Translate Driver.
But now we found from the translate button is clicked to the page is translated, there are many steps. If any step is returned, we are not be notified at all.

In order to fix the loose relationship, we create a responder inside the delegate.
So no matter what delegate is going to do, we will be notified, which keep infobar always consistent with delegate.

What's more, on some corner cases listed in bug/723426, the relationship between infobar and translate driver causes crash. And the new strong relationship will fix it.

BUG=720164,703887,723426, 724428
TBR=dfalcantara@chromium.org

Review-Url: https://codereview.chromium.org/2894553002
Cr-Commit-Position: refs/heads/master@{#473799}

chrome/browser/ui/android/infobars/translate_compact_infobar.cc

@@ -22,17 +22,12 @@ using base::android::ScopedJavaLocalRef;
 
 TranslateCompactInfoBar::TranslateCompactInfoBar(
     std::unique_ptr<translate::TranslateInfoBarDelegate> delegate)
-    : InfoBarAndroid(std::move(delegate)) {
-  // |translate_driver| must already be bound.
-  DCHECK(GetDelegate()->GetTranslateDriver());
-  translate_driver_ = static_cast<translate::ContentTranslateDriver*>(
-      GetDelegate()->GetTranslateDriver());
-  translate_driver_->AddObserver(this);
+    : InfoBarAndroid(std::move(delegate)), action_flags_(FLAG_NONE) {
+  GetDelegate()->SetObserver(this);
 }
 
 TranslateCompactInfoBar::~TranslateCompactInfoBar() {
-  DCHECK(translate_driver_);
-  translate_driver_->RemoveObserver(this);
+  GetDelegate()->SetObserver(nullptr);
 }
 
 ScopedJavaLocalRef<jobject> TranslateCompactInfoBar::CreateRenderInfoBar(
@@ -67,6 +62,7 @@ void TranslateCompactInfoBar::ProcessButton(int action) {
   // TODO(ramyasharma): Handle other button clicks.
   translate::TranslateInfoBarDelegate* delegate = GetDelegate();
   if (action == InfoBarAndroid::ACTION_TRANSLATE) {
+    action_flags_ |= FLAG_TRANSLATE;
     delegate->Translate();
     if (!delegate->ShouldAlwaysTranslate() && ShouldAutoAlwaysTranslate()) {
       JNIEnv* env = base::android::AttachCurrentThread();
@@ -74,6 +70,7 @@ void TranslateCompactInfoBar::ProcessButton(int action) {
                                                           GetJavaInfoBar());
     }
   } else if (action == InfoBarAndroid::ACTION_TRANSLATE_SHOW_ORIGINAL) {
+    action_flags_ |= FLAG_REVERT;
     delegate->RevertTranslation();
   } else if (action == InfoBarAndroid::ACTION_CANCEL) {
     delegate->TranslationDeclined();
@@ -119,14 +116,17 @@ void TranslateCompactInfoBar::ApplyBoolTranslateOption(
   translate::TranslateInfoBarDelegate* delegate = GetDelegate();
   if (option == TranslateUtils::OPTION_ALWAYS_TRANSLATE) {
     if (delegate->ShouldAlwaysTranslate() != value) {
+      action_flags_ |= FLAG_ALWAYS_TRANSLATE;
       delegate->ToggleAlwaysTranslate();
     }
   } else if (option == TranslateUtils::OPTION_NEVER_TRANSLATE) {
     if (value && delegate->IsTranslatableLanguageByPrefs()) {
+      action_flags_ |= FLAG_NEVER_LANGUAGE;
       delegate->ToggleTranslatableLanguageByPrefs();
     }
   } else if (option == TranslateUtils::OPTION_NEVER_TRANSLATE_SITE) {
     if (value && !delegate->IsSiteBlacklisted()) {
+      action_flags_ |= FLAG_NEVER_SITE;
       delegate->ToggleSiteBlacklist();
     }
   } else {
@@ -134,19 +134,6 @@ void TranslateCompactInfoBar::ApplyBoolTranslateOption(
   }
 }
 
-void TranslateCompactInfoBar::OnPageTranslated(
-    const std::string& original_lang,
-    const std::string& translated_lang,
-    translate::TranslateErrors::Type error_type) {
-  if (!owner())
-    return;  // We're closing; don't call anything, it might access the owner.
-
-  DCHECK(translate_driver_);
-  JNIEnv* env = base::android::AttachCurrentThread();
-  Java_TranslateCompactInfoBar_onPageTranslated(env, GetJavaInfoBar(),
-                                                error_type);
-}
-
 bool TranslateCompactInfoBar::ShouldAutoAlwaysTranslate() {
   translate::TranslateInfoBarDelegate* delegate = GetDelegate();
   return (delegate->GetTranslationAcceptedCount() == kAcceptCountThreshold);
@@ -163,6 +150,24 @@ translate::TranslateInfoBarDelegate* TranslateCompactInfoBar::GetDelegate() {
   return delegate()->AsTranslateInfoBarDelegate();
 }
 
+void TranslateCompactInfoBar::OnTranslateStepChanged(
+    translate::TranslateStep step,
+    translate::TranslateErrors::Type error_type) {
+  if (!owner())
+    return;  // We're closing; don't call anything.
+
+  if ((step == translate::TRANSLATE_STEP_AFTER_TRANSLATE) ||
+      (step == translate::TRANSLATE_STEP_TRANSLATE_ERROR)) {
+    JNIEnv* env = base::android::AttachCurrentThread();
+    Java_TranslateCompactInfoBar_onPageTranslated(env, GetJavaInfoBar(),
+                                                  error_type);
+  }
+}
+
+bool TranslateCompactInfoBar::IsDeclinedByUser() {
+  return action_flags_ == FLAG_NONE;
+};
+
 // Native JNI methods ---------------------------------------------------------
 
 // static

chrome/browser/ui/android/infobars/translate_compact_infobar.h

@@ -9,7 +9,9 @@
 #include "base/macros.h"
 #include "chrome/browser/translate/chrome_translate_client.h"
 #include "chrome/browser/ui/android/infobars/infobar_android.h"
-#include "components/translate/content/browser/content_translate_driver.h"
+#include "components/translate/core/browser/translate_infobar_delegate.h"
+#include "components/translate/core/browser/translate_step.h"
+#include "components/translate/core/common/translate_errors.h"
 
 namespace translate {
 class TranslateInfoBarDelegate;
@@ -17,7 +19,7 @@ class TranslateInfoBarDelegate;
 
 class TranslateCompactInfoBar
     : public InfoBarAndroid,
-      public translate::ContentTranslateDriver::Observer {
+      public translate::TranslateInfoBarDelegate::Observer {
  public:
   explicit TranslateCompactInfoBar(
       std::unique_ptr<translate::TranslateInfoBarDelegate> delegate);
@@ -44,10 +46,10 @@ class TranslateCompactInfoBar
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& obj);
 
-  // ContentTranslateDriver::Observer implementation.
-  void OnPageTranslated(const std::string& original_lang,
-                        const std::string& translated_lang,
-                        translate::TranslateErrors::Type error_type) override;
+  // TranslateInfoBarDelegate::Observer implementation.
+  void OnTranslateStepChanged(translate::TranslateStep step,
+                    translate::TranslateErrors::Type error_type) override;
+  bool IsDeclinedByUser() override;
 
  private:
   // InfoBarAndroid:
@@ -58,7 +60,19 @@ class TranslateCompactInfoBar
       const base::android::JavaRef<jobject>& java_info_bar) override;
 
   translate::TranslateInfoBarDelegate* GetDelegate();
-  translate::ContentTranslateDriver* translate_driver_;
+
+  // Bits for trace user actions.
+  unsigned int action_flags_;
+
+  // User action flags to record what the user has done in each session.
+  enum ActionFlag {
+    FLAG_NONE = 0,
+    FLAG_TRANSLATE = 1 << 0,
+    FLAG_REVERT = 1 << 1,
+    FLAG_ALWAYS_TRANSLATE = 1 << 2,
+    FLAG_NEVER_LANGUAGE = 1 << 3,
+    FLAG_NEVER_SITE = 1 << 4,
+  };
 
   // If number of consecutive translations is equal to this number, infobar will
   // automatically trigger "Always Translate".

components/translate/core/browser/translate_infobar_delegate.cc

@@ -101,12 +101,18 @@ void TranslateInfoBarDelegate::Create(
     old_infobar = infobar_manager->infobar_at(i);
     old_delegate = old_infobar->delegate()->AsTranslateInfoBarDelegate();
     if (old_delegate) {
-      if (!replace_existing_infobar || IsCompactUIEnabled())
+      if (!replace_existing_infobar)
         return;
       break;
     }
   }
 
+  // Try to reuse existing translate infobar delegate.
+  if (old_delegate && old_delegate->observer_) {
+    old_delegate->observer_->OnTranslateStepChanged(step, error_type);
+    return;
+  }
+
   // Add the new delegate.
   TranslateClient* translate_client = translate_manager->translate_client();
   std::unique_ptr<infobars::InfoBar> infobar(translate_client->CreateInfoBar(
@@ -124,6 +130,10 @@ bool TranslateInfoBarDelegate::IsCompactUIEnabled() {
   return base::FeatureList::IsEnabled(kTranslateCompactUI);
 }
 
+void TranslateInfoBarDelegate::SetObserver(Observer* observer) {
+  observer_ = observer;
+}
+
 void TranslateInfoBarDelegate::UpdateOriginalLanguage(
     const std::string& language_code) {
   ui_delegate_.UpdateOriginalLanguage(language_code);
@@ -361,7 +371,8 @@ TranslateInfoBarDelegate::TranslateInfoBarDelegate(
       translate_manager_(translate_manager),
       error_type_(error_type),
       prefs_(translate_manager->translate_client()->GetTranslatePrefs()),
-      triggered_from_menu_(triggered_from_menu) {
+      triggered_from_menu_(triggered_from_menu),
+      observer_(nullptr) {
   DCHECK_NE((step_ == translate::TRANSLATE_STEP_TRANSLATE_ERROR),
             (error_type_ == TranslateErrors::NONE));
   DCHECK(translate_manager_);
@@ -377,13 +388,15 @@ int TranslateInfoBarDelegate::GetIconId() const {
 }
 
 void TranslateInfoBarDelegate::InfoBarDismissed() {
-  if (step_ != translate::TRANSLATE_STEP_BEFORE_TRANSLATE)
-    return;
-  if (IsCompactUIEnabled())
-    return;
-  // The user closed the infobar without clicking the translate button.
-  TranslationDeclined();
-  UMA_HISTOGRAM_BOOLEAN("Translate.DeclineTranslateCloseInfobar", true);
+  bool declined = observer_
+                      ? observer_->IsDeclinedByUser()
+                      : (step_ == translate::TRANSLATE_STEP_BEFORE_TRANSLATE);
+
+  if (declined) {
+    // The user closed the infobar without clicking the translate button.
+    TranslationDeclined();
+    UMA_HISTOGRAM_BOOLEAN("Translate.DeclineTranslateCloseInfobar", true);
+  }
 }
 
 TranslateInfoBarDelegate*

components/translate/core/browser/translate_infobar_delegate.h

@@ -37,6 +37,19 @@ class TranslateManager;
 
 class TranslateInfoBarDelegate : public infobars::InfoBarDelegate {
  public:
+  // An observer to handle different translate steps' UI changes.
+  class Observer {
+   public:
+    // Handles UI changes on the translate step given.
+    virtual void OnTranslateStepChanged(translate::TranslateStep step,
+                              TranslateErrors::Type error_type){};
+    // Return whether user declined translate service.
+    virtual bool IsDeclinedByUser();
+
+   protected:
+    virtual ~Observer() {}
+  };
+
   static const size_t kNoIndex;
 
   ~TranslateInfoBarDelegate() override;
@@ -182,6 +195,9 @@ class TranslateInfoBarDelegate : public infobars::InfoBarDelegate {
   // May return NULL if the driver has been destroyed.
   TranslateDriver* GetTranslateDriver();
 
+  // Set a observer.
+  void SetObserver(Observer* observer);
+
  protected:
   TranslateInfoBarDelegate(
       const base::WeakPtr<TranslateManager>& translate_manager,
@@ -219,6 +235,10 @@ class TranslateInfoBarDelegate : public infobars::InfoBarDelegate {
   // (due to language detection, preferences...)
   bool triggered_from_menu_;
 
+  // A observer to handle front-end changes on different steps.
+  // It's only used when we try to reuse the existing UI.
+  Observer* observer_;
+
   DISALLOW_COPY_AND_ASSIGN(TranslateInfoBarDelegate);
 };