Replace polling with callbacks in translate_ios.js.

This should alleviate flaky errors when translating on iOS.

When translate is tapped, translate_ios.js polls translate.js to wait
for translate element ready and then later polls again to wait for the
translation result. Polling can often time out due to network slowness.

Bug: 869729
Change-Id: I7e0e46fe279571f21b3aa0dd18f241dc83afee90
Reviewed-on: https://chromium-review.googlesource.com/1178691Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Hiroshi Ichikawa <ichikawa@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: John Wu <jzw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584549}

components/translate/core/browser/resources/translate.js

@@ -102,14 +102,28 @@ cr.googleTranslate = (function() {
    */
   var endTime = 0.0;
 
+  /**
+   * Callback invoked when Translate Element's ready state is known.
+   * @type {Function}
+   */
+  var readyCallback;
+
+  /**
+   * Callback invoked when Translate Element's translation result is known.
+   * @type {Function}
+   */
+  var resultCallback;
+
   function checkLibReady() {
     if (lib.isAvailable()) {
       readyTime = performance.now();
       libReady = true;
+      invokeReadyCallback();
       return;
     }
     if (checkReadyCount++ > 5) {
       errorCode = ERROR['TRANSLATION_TIMEOUT'];
+      invokeReadyCallback();
       return;
     }
     setTimeout(checkLibReady, 100);
@@ -123,16 +137,52 @@ cr.googleTranslate = (function() {
       errorCode = ERROR['TRANSLATION_ERROR'];
       // We failed to translate, restore so the page is in a consistent state.
       lib.restore();
+      invokeResultCallback();
     } else if (typeof opt_error == 'number' && opt_error != 0) {
       errorCode = TRANSLATE_ERROR_TO_ERROR_CODE_MAP[opt_error];
       lib.restore();
+      invokeResultCallback();
     }
-    if (finished)
+    if (finished) {
       endTime = performance.now();
+      invokeResultCallback();
+    }
+  }
+
+  function invokeReadyCallback() {
+    if (readyCallback) {
+      readyCallback();
+    }
+  }
+
+  function invokeResultCallback() {
+    if (resultCallback) {
+      resultCallback();
+    }
   }
 
   // Public API.
   return {
+    /**
+     * Setter for readyCallback. No op if already set.
+     * @param {Function} callback The function to be invoked.
+     */
+    set readyCallback(callback) {
+      if (!readyCallback) {
+        readyCallback = callback;
+      }
+    },
+
+    /**
+     * Setter for resultCallback. No op if already set.
+     * @param {Function} callback The function to be invoked.
+     */
+    set resultCallback(callback) {
+      if (!resultCallback) {
+        resultCallback = callback;
+      }
+    },
+
     /**
      * Whether the library is ready.
      * The translate function should only be called when |libReady| is true.
@@ -235,6 +285,7 @@ cr.googleTranslate = (function() {
       } catch (err) {
         console.error('Translate: ' + err);
         errorCode = ERROR['UNEXPECTED_SCRIPT_ERROR'];
+        invokeResultCallback();
         return false;
       }
       return true;
@@ -270,6 +321,7 @@ cr.googleTranslate = (function() {
         translateApiKey = undefined;
         serverParams = undefined;
         gtTimeInfo = undefined;
+        invokeReadyCallback();
         return;
       }
       // The TranslateService is not available immediately as it needs to start

components/translate/ios/browser/ios_translate_driver.h

@@ -77,13 +77,6 @@ class IOSTranslateDriver : public TranslateDriver,
                              int page_seq_no,
                              const std::string& original_page_language,
                              double translation_time);
-  // Checks if the current running page translation is finished or errored and
-  // notifies the browser accordingly.  If the translation has not terminated,
-  // posts a task to check again later.
-  // Similar to TranslateHelper::CheckTranslateStatus on desktop.
-  void CheckTranslateStatus(const std::string& source_language,
-                            const std::string& target_language,
-                            int page_seq_no);
 
   // Returns true if the user has not navigated away and the the page is not
   // being destroyed.

components/translate/ios/browser/ios_translate_driver.mm

@@ -37,11 +37,6 @@
 namespace translate {
 
 namespace {
-// The delay we wait in milliseconds before checking whether the translation has
-// finished.
-// Note: This should be kept in sync with the constant of the same name in
-// translate_ios.js.
-const int kTranslateStatusCheckDelayMs = 400;
 // Language name passed to the Translate element for it to detect the language.
 const char kAutoDetectionLanguage[] = "auto";
 
@@ -225,15 +220,6 @@ void IOSTranslateDriver::TranslationDidSucceed(
                                      translate_errors);
 }
 
-void IOSTranslateDriver::CheckTranslateStatus(
-    const std::string& source_language,
-    const std::string& target_language,
-    int page_seq_no) {
-  if (!IsPageValid(page_seq_no))
-    return;
-  translate_controller_->CheckTranslateStatus();
-}
-
 bool IOSTranslateDriver::IsPageValid(int page_seq_no) const {
   bool user_navigated_away = page_seq_no != page_seq_no_;
   return !user_navigated_away && web_state_;
@@ -260,12 +246,6 @@ void IOSTranslateDriver::OnTranslateScriptReady(bool success,
                            ? source_language_
                            : kAutoDetectionLanguage;
   translate_controller_->StartTranslation(source_language_, target_language_);
-  // Check the status of the translation -- after a delay.
-  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, base::Bind(&IOSTranslateDriver::CheckTranslateStatus,
-                            weak_method_factory_.GetWeakPtr(), source_language_,
-                            target_language_, pending_page_seq_no_),
-      base::TimeDelta::FromMilliseconds(kTranslateStatusCheckDelayMs));
 }
 
 void IOSTranslateDriver::OnTranslateComplete(

components/translate/ios/browser/js_translate_manager.h

@@ -24,14 +24,6 @@
 // after the injection.
 @property(nonatomic, copy) NSString* script;
 
-// Injects JS to constantly check if the translate script is ready and informs
-// the Obj-C side when it is.
-- (void)injectWaitUntilTranslateReadyScript;
-
-// After a translation has been initiated, injects JS to check if the
-// translation has finished/failed and informs the Obj-C when it is.
-- (void)injectTranslateStatusScript;
-
 // Starts translation of the page from |source| language to |target| language.
 // Equivalent to TranslateHelper::StartTranslation().
 - (void)startTranslationFrom:(const std::string&)source

components/translate/ios/browser/js_translate_manager.mm

@@ -47,16 +47,6 @@
   _translationScript = [script copy];
 }
 
-- (void)injectWaitUntilTranslateReadyScript {
-  [self.receiver executeJavaScript:@"__gCrWeb.translate.checkTranslateReady()"
-                 completionHandler:nil];
-}
-
-- (void)injectTranslateStatusScript {
-  [self.receiver executeJavaScript:@"__gCrWeb.translate.checkTranslateStatus()"
-                 completionHandler:nil];
-}
-
 - (void)startTranslationFrom:(const std::string&)source
                           to:(const std::string&)target {
   NSString* script =

components/translate/ios/browser/resources/translate_ios.js

@@ -3,89 +3,33 @@
 // found in the LICENSE file.
 
 /**
- * @fileoverview Installs Translate management functions on the __gCrWeb object.
+ * @fileoverview Installs iOS Translate callbacks on cr.googleTranslate.
  *
  * TODO(crbug.com/659442): Enable checkTypes, checkVars errors for this file.
  * @suppress {checkTypes, checkVars}
  */
 
-__gCrWeb.translate = {};
-
-// Store translate namespace object in a global __gCrWeb object referenced by a
-// string, so it does not get renamed by closure compiler during the
-// minification.
-__gCrWeb['translate'] = __gCrWeb.translate;
-
 (function() {
 /**
- * The delay a wait performed (in milliseconds) before checking whether the
- * translation has finished.
- * @type {number}
- */
-__gCrWeb.translate.TRANSLATE_STATUS_CHECK_DELAY = 400;
-
-/**
- * The delay in milliseconds that we'll wait to check if a page has finished
- * loading before attempting a translation.
- * @type {number}
- */
-__gCrWeb.translate.TRANSLATE_LOAD_CHECK_DELAY = 150;
-
-/**
- * The maximum number of times a check is performed to see if the translate
- * library injected in the page is ready.
- * @type {number}
+ * Sets a callback to inform host of the ready state of the translate element.
  */
-__gCrWeb.translate.MAX_TRANSLATE_INIT_CHECK_ATTEMPTS = 5;
-
-// The number of times polling for the ready status of the translate script has
-//  been performed.
-var translationAttemptCount = 0;
-
-/**
- * Polls every TRANSLATE_LOAD_CHECK_DELAY milliseconds to check if the translate
- * script is ready and informs the host when it is.
- */
-__gCrWeb.translate['checkTranslateReady'] = function() {
-  translationAttemptCount += 1;
-  if (cr.googleTranslate.libReady) {
-    translationAttemptCount = 0;
+cr.googleTranslate.readyCallback = function() {
   __gCrWeb.message.invokeOnHost({
       'command': 'translate.ready',
-        'timeout': false,
+      'timeout': cr.googleTranslate.error,
       'loadTime': cr.googleTranslate.loadTime,
       'readyTime': cr.googleTranslate.readyTime});
-  } else if (translationAttemptCount >=
-             __gCrWeb.translate.MAX_TRANSLATE_INIT_CHECK_ATTEMPTS) {
-    __gCrWeb.message.invokeOnHost({
-        'command': 'translate.ready',
-        'timeout': true});
-  } else {
-    // The translation is still pending, check again later.
-    window.setTimeout(__gCrWeb.translate.checkTranslateReady,
-                      __gCrWeb.translate.TRANSLATE_LOAD_CHECK_DELAY);
-  }
 }
 
 /**
- * Polls every TRANSLATE_STATUS_CHECK_DELAY milliseconds to check if translate
- * is ready and informs the host when it is.
+ * Sets a callback to inform host of the result of translation.
  */
-__gCrWeb.translate['checkTranslateStatus'] = function() {
-  if (cr.googleTranslate.error) {
-    __gCrWeb.message.invokeOnHost({'command': 'translate.status',
-                                   'success': false});
-  } else if (cr.googleTranslate.finished) {
+cr.googleTranslate.resultCallback = function() {
   __gCrWeb.message.invokeOnHost({
       'command': 'translate.status',
-        'success': true,
+      'success': !cr.googleTranslate.error,
       'originalPageLanguage': cr.googleTranslate.sourceLang,
       'translationTime': cr.googleTranslate.translationTime});
-  } else {
-    // The translation is still pending, check again later.
-    window.setTimeout(__gCrWeb.translate.checkTranslateStatus,
-                      __gCrWeb.translate.TRANSLATE_STATUS_CHECK_DELAY);
-  }
 }
 
 }());  // anonymous function

components/translate/ios/browser/translate_controller.h

@@ -62,10 +62,6 @@ class TranslateController : public web::WebStateObserver {
   void StartTranslation(const std::string& source_language,
                         const std::string& target_language);
 
-  // Checks the translation status and calls the observer when it is done.
-  // This method must be called after StartTranslation().
-  void CheckTranslateStatus();
-
   // Changes the JsTranslateManager used by this TranslateController.
   // Only used for testing.
   void SetJsTranslateManagerForTesting(JsTranslateManager* manager);

components/translate/ios/browser/translate_controller.mm

@@ -50,7 +50,6 @@ void TranslateController::InjectTranslateScript(
     const std::string& translate_script) {
   [js_manager_ setScript:base::SysUTF8ToNSString(translate_script)];
   [js_manager_ inject];
-  [js_manager_ injectWaitUntilTranslateReadyScript];
 }
 
 void TranslateController::RevertTranslation() {
@@ -62,10 +61,6 @@ void TranslateController::StartTranslation(const std::string& source_language,
   [js_manager_ startTranslationFrom:source_language to:target_language];
 }
 
-void TranslateController::CheckTranslateStatus() {
-  [js_manager_ injectTranslateStatusScript];
-}
-
 void TranslateController::SetJsTranslateManagerForTesting(
     JsTranslateManager* manager) {
   js_manager_.reset(manager);