Translate: New Bubble UX (for the view toolkit)

chrome/browser/translate/translate_manager.cc

Index: chrome/browser/translate/translate_manager.cc
diff --git a/chrome/browser/translate/translate_manager.cc b/chrome/browser/translate/translate_manager.cc
index 2751f170e25ebf4844ebcb44aa62996b093167a7..2e05135eae95742ac6fdfcc25eb372d48aff43de 100644
--- a/chrome/browser/translate/translate_manager.cc
+++ b/chrome/browser/translate/translate_manager.cc
@@ -32,7 +32,9 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/translate/translate_bubble.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
@@ -61,6 +63,8 @@ using content::NavigationController;
 using content::NavigationEntry;
 using content::WebContents;
 
+namespace {
+
 const char kReportLanguageDetectionErrorURL[] =
     "https://translate.google.com/translate_error?client=cr&action=langidc";
 
@@ -75,6 +79,22 @@ const char kUrlQueryName[] = "u";
 // loading before giving up the translation
 const int kMaxTranslateLoadCheckAttempts = 20;
 
+bool IsEnabledTranslateNewUX() {

[Takashi Toyoshima, 2013/10/08 14:54:09]

IsTranslateNewUXEnabled() sounds better.

Just an idea.
By the way, TranslateManager is a singleton. So, I guess we don't have to check
a flag for each. Can we check all flags TranslateManager need when the singleton
is created, and cache them as member variables?
It's ok to forget this idea in this CL.

+  return CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableTranslateNewUX);
+}
+
+void ShowBubble(WebContents* web_contents, TranslateBubble::Type type) {
+  // TODO(hajimehoshi): Show bubble only when needed.

[Takashi Toyoshima, 2013/10/08 14:54:09]

What do you mean by this TODO?

[hajimehoshi, 2013/10/10 11:07:10]

This comment means nothing. I'll remove this.

+  Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
+  DCHECK(browser != NULL);

[Takashi Toyoshima, 2013/10/08 14:54:09]

Do you check if these DCHECK are always correct?
Some classes have an implementation for browser being NULL. I'm afraid it might
happen here too.

[hajimehoshi, 2013/10/10 11:07:10]

I found these assertion failed on TranslateManagerBrowserTest. I'll remove them.

+  BrowserWindow* window = browser->window();
+  DCHECK(window != NULL);
+  window->ShowTranslateBubble(web_contents, type);
+}
+
+}  // namespace
+
 TranslateManager::~TranslateManager() {
   weak_method_factory_.InvalidateWeakPtrs();
 }
@@ -303,6 +323,11 @@ TranslateManager::TranslateManager()
 
 void TranslateManager::InitiateTranslation(WebContents* web_contents,
                                            const std::string& page_lang) {
+  TranslateTabHelper* translate_tab_helper =
+      TranslateTabHelper::FromWebContents(web_contents);
+  if (!translate_tab_helper)
+    return;
+
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
   Profile* original_profile = profile->GetOriginalProfile();
@@ -396,12 +421,8 @@ void TranslateManager::InitiateTranslation(WebContents* web_contents,
     }
   }
 
-  TranslateTabHelper* translate_tab_helper =
-      TranslateTabHelper::FromWebContents(web_contents);
-  if (!translate_tab_helper)
-    return;
-  std::string auto_translate_to =
-      translate_tab_helper->language_state().AutoTranslateTo();
+  LanguageState& language_state = translate_tab_helper->language_state();
+  std::string auto_translate_to = language_state.AutoTranslateTo();
   if (!auto_translate_to.empty()) {
     // This page was navigated through a click from a translated page.
     TranslateBrowserMetrics::ReportInitiationStatus(
@@ -410,13 +431,20 @@ void TranslateManager::InitiateTranslation(WebContents* web_contents,
     return;
   }
 
-  // Prompts the user if he/she wants the page translated.
   TranslateBrowserMetrics::ReportInitiationStatus(
       TranslateBrowserMetrics::INITIATION_STATUS_SHOW_INFOBAR);
-  TranslateInfoBarDelegate::Create(
-      false, InfoBarService::FromWebContents(web_contents),
-      TranslateInfoBarDelegate::BEFORE_TRANSLATE, language_code, target_lang,
-      TranslateErrors::NONE, profile->GetPrefs(), ShortcutConfig());
+
+  if (IsEnabledTranslateNewUX()) {
+    language_state.SetIsTranslateSuggested(true);
+    if (language_state.IsLanguageChanged())
+      ShowBubble(web_contents, TranslateBubble::BEFORE_TRANSLATE);
+  } else {
+    // Prompts the user if he/she wants the page translated.
+    TranslateInfoBarDelegate::Create(
+        false, InfoBarService::FromWebContents(web_contents),
+        TranslateInfoBarDelegate::BEFORE_TRANSLATE, language_code, target_lang,
+        TranslateErrors::NONE, profile->GetPrefs(), ShortcutConfig());
+  }
 }
 
 void TranslateManager::InitiateTranslationPosted(int process_id,
@@ -468,12 +496,16 @@ void TranslateManager::TranslatePage(WebContents* web_contents,
   if (!IsSupportedLanguage(source_lang))
     source_lang = std::string(translate::kUnknownLanguageCode);
 
-  Profile* profile =
-      Profile::FromBrowserContext(web_contents->GetBrowserContext());
-  TranslateInfoBarDelegate::Create(
-      true, InfoBarService::FromWebContents(web_contents),
-      TranslateInfoBarDelegate::TRANSLATING, source_lang, target_lang,
-      TranslateErrors::NONE, profile->GetPrefs(), ShortcutConfig());
+  if (IsEnabledTranslateNewUX()) {
+    ShowBubble(web_contents, TranslateBubble::TRANSLATING);
+  } else {
+    Profile* profile =
+        Profile::FromBrowserContext(web_contents->GetBrowserContext());
+    TranslateInfoBarDelegate::Create(
+        true, InfoBarService::FromWebContents(web_contents),
+        TranslateInfoBarDelegate::TRANSLATING, source_lang, target_lang,
+        TranslateErrors::NONE, profile->GetPrefs(), ShortcutConfig());
+  }
 
   DCHECK(script_.get() != NULL);
 
@@ -585,15 +617,23 @@ void TranslateManager::PageTranslated(WebContents* web_contents,
     details->error_type = TranslateErrors::UNSUPPORTED_LANGUAGE;
   }
 
-  PrefService* prefs = Profile::FromBrowserContext(
-      web_contents->GetBrowserContext())->GetPrefs();
-  TranslateInfoBarDelegate::Create(
-      true, InfoBarService::FromWebContents(web_contents),
-      (details->error_type == TranslateErrors::NONE) ?
-          TranslateInfoBarDelegate::AFTER_TRANSLATE :
-          TranslateInfoBarDelegate::TRANSLATION_ERROR,
-      details->source_language, details->target_language, details->error_type,
-      prefs, ShortcutConfig());
+  if (IsEnabledTranslateNewUX()) {
+    TranslateBubble::Type type =
+        (details->error_type == TranslateErrors::NONE) ?
+        TranslateBubble::AFTER_TRANSLATE :
+        TranslateBubble::ERROR;
+    ShowBubble(web_contents, type);
+  } else {
+    PrefService* prefs = Profile::FromBrowserContext(
+        web_contents->GetBrowserContext())->GetPrefs();
+    TranslateInfoBarDelegate::Create(
+        true, InfoBarService::FromWebContents(web_contents),
+        (details->error_type == TranslateErrors::NONE) ?
+        TranslateInfoBarDelegate::AFTER_TRANSLATE :
+        TranslateInfoBarDelegate::TRANSLATION_ERROR,
+        details->source_language, details->target_language, details->error_type,
+        prefs, ShortcutConfig());
+  }
 
   if (details->error_type != TranslateErrors::NONE &&
       !web_contents->GetBrowserContext()->IsOffTheRecord()) {
@@ -651,13 +691,17 @@ void TranslateManager::OnTranslateScriptFetchComplete(
       DoTranslatePage(web_contents, translate_script,
                       request.source_lang, request.target_lang);
     } else {
-      Profile* profile =
-          Profile::FromBrowserContext(web_contents->GetBrowserContext());
-      TranslateInfoBarDelegate::Create(
-          true, InfoBarService::FromWebContents(web_contents),
-          TranslateInfoBarDelegate::TRANSLATION_ERROR, request.source_lang,
-          request.target_lang, TranslateErrors::NETWORK, profile->GetPrefs(),
-          ShortcutConfig());
+      if (IsEnabledTranslateNewUX()) {
+        ShowBubble(web_contents, TranslateBubble::ERROR);
+      } else {
+        Profile* profile =
+            Profile::FromBrowserContext(web_contents->GetBrowserContext());
+        TranslateInfoBarDelegate::Create(
+            true, InfoBarService::FromWebContents(web_contents),
+            TranslateInfoBarDelegate::TRANSLATION_ERROR, request.source_lang,
+            request.target_lang, TranslateErrors::NETWORK, profile->GetPrefs(),
+            ShortcutConfig());
+      }
 
       if (!web_contents->GetBrowserContext()->IsOffTheRecord()) {
         TranslateErrorDetails error_details;