Implemented new Translate API for purely Objective-C clients.

ios/web_view/public/cwv_translation_controller.h

Index: ios/web_view/public/cwv_translation_controller.h
diff --git a/ios/web_view/public/cwv_translation_controller.h b/ios/web_view/public/cwv_translation_controller.h
new file mode 100644
index 0000000000000000000000000000000000000000..fa0e700822763001da0d71e41682d2a2b4bdc857
--- /dev/null
+++ b/ios/web_view/public/cwv_translation_controller.h
@@ -0,0 +1,89 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef IOS_WEB_VIEW_PUBLIC_CWV_TRANSLATION_CONTROLLER_H
+#define IOS_WEB_VIEW_PUBLIC_CWV_TRANSLATION_CONTROLLER_H
+
+#import <ChromeWebView/cwv_export.h>
+#import <Foundation/Foundation.h>
+
+@class CWVTranslationController;
+@class CWVLanguageDetectionResult;
+@class CWVTranslationLanguage;
+
+// Possible errors during translation.

[Eugene But (OOO till 7-30), 2017/04/26 20:15:30]

s/errors/error codes ?

[jzw1, 2017/04/27 04:54:02]

Done.

+typedef NS_ENUM(NSInteger, CWVTranslationError) {

[Eugene But (OOO till 7-30), 2017/04/26 20:15:31]

Is there a need for enum? Should these constants be just integers?

[jzw1, 2017/04/27 04:54:02]

I'm just copying the error codes over from translate::TranslateErrors::Type

[Eugene But (OOO till 7-30), 2017/04/27 13:08:08]

Looks like iOS SDK do not use named enums and use something like this:
|NS_ENUM(NSInteger)|. Do you want to use their approach?
We don't have a goal to copy translate::TranslateErrors::Type. Right?

[jzw1, 2017/04/28 02:34:53]

I'll get rid of the name. Since any of these errors could be returned by the
underlying C++ code, we should just pass them on to the client right?

[Eugene But (OOO till 7-30), 2017/04/28 03:07:28]

Sure.

+  // No errors.
+  CWVTranslationErrorNone = 0,

[Eugene But (OOO till 7-30), 2017/04/26 20:15:30]

Is there a need for this item? This error code can never be used, right?

[jzw1, 2017/04/27 04:54:02]

I'm just copying the error codes over from translate::TranslateErrors::Type

[Eugene But (OOO till 7-30), 2017/04/27 13:08:08]

That code is C++ code and 0 in C++ means no error. In Objective-C nil NSError is
used for non error cases so it sounds like we don't need this error code.

[jzw1, 2017/04/28 02:34:53]

Done.

+  // No connectivity.
+  CWVTranslationErrorNetwork,

[Eugene But (OOO till 7-30), 2017/04/26 20:15:31]

Normally large negative numbers are used for error codes. Do you want to use
one?

[jzw1, 2017/04/27 04:54:02]

I'm just copying the error codes over from translate::TranslateErrors::Type

[Eugene But (OOO till 7-30), 2017/04/27 13:08:08]

If the goal is to use the same error codes as translate::TranslateErrors::Type,
then I think we can't just assume that translate::TranslateErrors::Type enum is
stable (i.e. someone can add an error code to translate::TranslateErrors::Type
and it will break this code). 

How about doing this:

in the header:
extern NSInteger CWVTranslationErrorInitializationError;

in the implementation
NSInteger CWVTranslationErrorInitializationError =
translate::TranslateErrors::Type::INITIALIZATION_ERROR

[jzw1, 2017/04/28 02:34:53]

Good point. I'll make this change.

+  // The translation script failed to initialize.
+  CWVTranslationErrorInitializationError,
+  // The page's language could not be detected.
+  CWVTranslationErrorUnknownLanguage,
+  // The server detected a language that the browser
+  // does not know.
+  CWVTranslationErrorUnsupportedLanguage,
+  // The original and target languages are the same.
+  CWVTranslationErrorIdenticalLanguages,
+  // An error was reported by the translation script
+  // during translation.
+  CWVTranslationErrorTranslationError,
+  // The library doesn't finish the translation.
+  CWVTranslationErrorTranslationTimeout,
+  // The library raises an unexpected exception.
+  CWVTranslationErrorUnexpectedScriptError,
+  // The library is blocked because of bad origin.
+  CWVTranslationErrorBadOrigin,
+  // Loader fails to load a dependent JavaScript.
+  CWVTranslationErrorScriptLoadError
+};
+
+// Updates delegate on translation progress.
+CWV_EXPORT
+@protocol CWVTranslationControllerDelegate<NSObject>

[Hiroshi Ichikawa, 2017/04/26 08:40:57]

How about marking this protocol as @optional, and call each method only if it is
defined? I guess it's a common pattern of delegates?

[Eugene But (OOO till 7-30), 2017/04/26 20:15:31]

+1

[jzw1, 2017/04/27 04:54:01]

Done.

+
+// Called when language detection is completed. Contains information on the
+// detected language, the assumed target language for translation, as well as
+// all languages supported by this translation controller.
+// |error| will be filled if language detection failed.

[Eugene But (OOO till 7-30), 2017/04/26 20:15:31]

Do you want to create a constant for Error Domain?

[jzw1, 2017/04/27 04:54:02]

Done.

+- (void)translationController:(CWVTranslationController*)controller
+    didFinishLanguageDetectionWithResult:(CWVLanguageDetectionResult*)result
+                                   error:(NSError*)error;
+
+// Called when a translation is in progress. Used to show loading indicator.

[Eugene But (OOO till 7-30), 2017/04/26 20:15:31]

s/is in progress/has started ?

[Eugene But (OOO till 7-30), 2017/04/26 20:15:31]

Please do not make assumptions how this is used.

[jzw1, 2017/04/27 04:54:02]

Done.

[jzw1, 2017/04/27 04:54:02]

Done.

+- (void)translationController:(CWVTranslationController*)controller
+    didStartTranslationFromLanguage:(CWVTranslationLanguage*)sourceLanguage
+                         toLanguage:(CWVTranslationLanguage*)targetLanguage;
+
+// Called when translation finishes. |error| will be filled if it failed.
+- (void)translationController:(CWVTranslationController*)controller
+    didFinishTranslationFromLanguage:(CWVTranslationLanguage*)sourceLanguage
+                          toLanguage:(CWVTranslationLanguage*)targetLanguage
+                               error:(NSError*)error;
+
+@end
+
+// Exposes everything needed for translation features.
+CWV_EXPORT
+@interface CWVTranslationController : NSObject
+
+- (instancetype)init NS_UNAVAILABLE;
+
+// Delegate to receive translation callbacks.
+@property(nonatomic, weak) id<CWVTranslationControllerDelegate> delegate;
+
+// Begins translation on the current page from |sourceLanguage| to

[Hiroshi Ichikawa, 2017/04/26 08:40:57]

Can you add a comment that this must not be called before
-translationController:didFinishLanguageDetectionWithResult:error: is called?
Also, can you document what happens if I call it before? Error? No-op?

Also, can you document whether we can call this method twice before the first
one completes? It can be TODO if you are not sure.

[Hiroshi Ichikawa, 2017/04/27 05:35:42]

Reminder: This hasn't been resolved yet.

[jzw1, 2017/04/27 07:03:18]

Done.

[Hiroshi Ichikawa, 2017/04/27 07:17:23]

Looks like this part of the comment hasn't been resolved yet:
the first

[jzw1, 2017/04/27 07:22:02]

Done.

+// |targetLanguage|.

[Eugene But (OOO till 7-30), 2017/04/26 20:15:31]

Could you please specify that |targetLanguage| should be in supported languages.

[jzw1, 2017/04/27 04:54:02]

Done.

+- (void)translatePageFromLanguage:(CWVTranslationLanguage*)sourceLanguage
+                       toLanguage:(CWVTranslationLanguage*)targetLanguage;
+
+// Reverts any translations done back to the original page language.

[Hiroshi Ichikawa, 2017/04/26 08:40:57]

Can you add a comment that this must not be called before
-translationController:didFinishLanguageDetectionWithResult:error: is called?
Also, can you document what happens if I call it before? Error? No-op?

[jzw1, 2017/04/27 04:54:02]

I'll investigate this soon after I build out some more UI to test this.

[Hiroshi Ichikawa, 2017/04/27 05:35:42]

Anyway this must not be called before
-translationController:didFinishLanguageDetectionWithResult:error: is called,
right? Can you comment it?

[jzw1, 2017/04/27 07:03:18]

Done.

+// Note that the original page language  may be different from |sourceLanguage|
+// passed to |translatePageFromLanguage:toLanguage:| above.

[Eugene But (OOO till 7-30), 2017/04/26 20:15:30]

What happens if this is called for a page which has not been translated?

[jzw1, 2017/04/27 04:54:02]

I'll report back in a future patch

[Hiroshi Ichikawa, 2017/04/27 05:35:42]

Can you add a TODO comment for it?

[jzw1, 2017/04/27 07:03:18]

Done.

+- (void)revertTranslation;
+
+@end
+
+#endif  // IOS_WEB_VIEW_PUBLIC_CWV_TRANSLATION_CONTROLLER_H