Video Tutorials : Fixed default language for user

This CL fixes two issues.
1 - The supported languages are converted from array to list at JNI
boundary.
2 - The preferred language was always falling back to the default
language from finch, which caused the language picker to never show up.
So, in the new fix, the preferred language will be empty if the user
hasn't set yet. However, showing the tutorial promos on the UI needs a
language. For that language, we will use a default language from finch.

Bug: 1142642
Change-Id: I3126b0d1bd815055ccda7a77903ca05cca72b75b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500266
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820997}

chrome/browser/video_tutorials/android/java/src/org/chromium/chrome/browser/video_tutorials/VideoTutorialService.java

@@ -29,7 +29,8 @@ public interface VideoTutorialService {
     List<String> getSupportedLanguages();
 
     /**
-     * @return The user's language of choice for watching the video tutorials.
+     * @return The user's language of choice for watching the video tutorials, or null user hasn't
+     *         set it yet.
      */
     String getPreferredLocale();
 

chrome/browser/video_tutorials/internal/android/java/src/org/chromium/chrome/browser/video_tutorials/bridges/VideoTutorialServiceBridge.java

@@ -12,6 +12,7 @@ import org.chromium.chrome.browser.video_tutorials.FeatureType;
 import org.chromium.chrome.browser.video_tutorials.Tutorial;
 import org.chromium.chrome.browser.video_tutorials.VideoTutorialService;
 
+import java.util.Arrays;
 import java.util.List;
 
 /**
@@ -47,8 +48,9 @@ public class VideoTutorialServiceBridge implements VideoTutorialService {
     @Override
     public List<String> getSupportedLanguages() {
         if (mNativeVideoTutorialServiceBridge == 0) return null;
-        return VideoTutorialServiceBridgeJni.get().getSupportedLanguages(
+        String[] languages = VideoTutorialServiceBridgeJni.get().getSupportedLanguages(
                 mNativeVideoTutorialServiceBridge, this);
+        return Arrays.asList(languages);
     }
 
     @Override
@@ -76,7 +78,7 @@ public class VideoTutorialServiceBridge implements VideoTutorialService {
                 Callback<List<Tutorial>> callback);
         void getTutorial(long nativeVideoTutorialServiceBridge, VideoTutorialServiceBridge caller,
                 int feature, Callback<Tutorial> callback);
-        List<String> getSupportedLanguages(
+        String[] getSupportedLanguages(
                 long nativeVideoTutorialServiceBridge, VideoTutorialServiceBridge caller);
         String getPreferredLocale(
                 long nativeVideoTutorialServiceBridge, VideoTutorialServiceBridge caller);

chrome/browser/video_tutorials/internal/android/java/src/org/chromium/chrome/browser/video_tutorials/player/VideoPlayerMediator.java

@@ -5,6 +5,7 @@
 package org.chromium.chrome.browser.video_tutorials.player;
 
 import android.content.Context;
+import android.text.TextUtils;
 
 import org.chromium.base.Callback;
 import org.chromium.chrome.browser.video_tutorials.Language;
@@ -92,7 +93,7 @@ class VideoPlayerMediator implements PlaybackStateObserver.Observer {
     void playVideoTutorial(Tutorial tutorial) {
         mTutorial = tutorial;
 
-        if (mVideoTutorialService.getPreferredLocale() == null) {
+        if (TextUtils.isEmpty(mVideoTutorialService.getPreferredLocale())) {
             mModel.set(VideoPlayerProperties.SHOW_LANGUAGE_PICKER, true);
             mLanguagePicker.showLanguagePicker(this::onLanguageSelected, mCloseCallback);
         } else {

chrome/browser/video_tutorials/internal/android/video_tutorial_service_bridge.cc

@@ -90,7 +90,8 @@ void VideoTutorialServiceBridge::GetTutorial(
                      ScopedJavaGlobalRef<jobject>(jcallback)));
 }
 
-ScopedJavaLocalRef<jobject> VideoTutorialServiceBridge::GetSupportedLanguages(
+ScopedJavaLocalRef<jobjectArray>
+VideoTutorialServiceBridge::GetSupportedLanguages(
     JNIEnv* env,
     const JavaParamRef<jobject>& jcaller) {
   return base::android::ToJavaArrayOfStrings(
@@ -100,8 +101,11 @@ ScopedJavaLocalRef<jobject> VideoTutorialServiceBridge::GetSupportedLanguages(
 ScopedJavaLocalRef<jstring> VideoTutorialServiceBridge::GetPreferredLocale(
     JNIEnv* env,
     const JavaParamRef<jobject>& jcaller) {
-  std::string locale = video_tutorial_service_->GetPreferredLocale();
-  return base::android::ConvertUTF8ToJavaString(env, locale);
+  base::Optional<std::string> locale =
+      video_tutorial_service_->GetPreferredLocale();
+  return locale.has_value()
+             ? base::android::ConvertUTF8ToJavaString(env, locale.value())
+             : ScopedJavaLocalRef<jstring>();
 }
 
 void VideoTutorialServiceBridge::SetPreferredLocale(

chrome/browser/video_tutorials/internal/android/video_tutorial_service_bridge.h

@@ -37,7 +37,7 @@ class VideoTutorialServiceBridge : public base::SupportsUserData::Data {
                    const JavaParamRef<jobject>& jcaller,
                    jint j_feature,
                    const JavaParamRef<jobject>& jcallback);
-  ScopedJavaLocalRef<jobject> GetSupportedLanguages(
+  ScopedJavaLocalRef<jobjectArray> GetSupportedLanguages(
       JNIEnv* env,
       const JavaParamRef<jobject>& jcaller);
   ScopedJavaLocalRef<jstring> GetPreferredLocale(

chrome/browser/video_tutorials/internal/config.cc

@@ -17,7 +17,7 @@ constexpr char kDefaultBaseURL[] =
 // Default URL string for GetTutorials RPC.
 constexpr char kDefaultGetTutorialsPath[] = "/v1/videotutorials";
 
-// Hindi is the default locale.
+// The default locale.
 constexpr char kDefaultPreferredLocale[] = "en";
 
 // Finch parameter key for base server URL to retrieve the tutorials.

chrome/browser/video_tutorials/internal/config.h

@@ -25,7 +25,8 @@ extern const char kBaseURLKey[];
 // Finch parameter key for the default preferred locale.
 extern const char kPreferredLocaleConfigKey[];
 
-// Default preferred locale setting before users pick.
+// Default preferred locale setting before user picks the language.
+// This will be used as the language for the video tutorial promo cards.
 extern const char kDefaultPreferredLocale[];
 
 // Finch parameter key for the fetch frequency to retrieve the tutorials.

chrome/browser/video_tutorials/internal/tutorial_manager.h

@@ -25,7 +25,7 @@ class TutorialManager {
   virtual const std::vector<std::string>& GetSupportedLanguages() = 0;
 
   // Returns the preferred locale for the video tutorials.
-  virtual std::string GetPreferredLocale() = 0;
+  virtual base::Optional<std::string> GetPreferredLocale() = 0;
 
   // Sets the user preferred locale for watching the video tutorials. This
   // doesn't update the cached tutorials. GetTutorials must be called for the

chrome/browser/video_tutorials/internal/tutorial_manager_impl.cc

@@ -35,8 +35,13 @@ void TutorialManagerImpl::GetTutorials(GetTutorialsCallback callback) {
     return;
   }
 
-  // Find the data from cache.
-  std::string locale = GetPreferredLocale();
+  // Find the data from cache. If the preferred locale is not set, use a default
+  // locale value to show the tutorial promos. Users will be asked again to
+  // confirm their language before the video starts.
+  base::Optional<std::string> preferred_locale = GetPreferredLocale();
+  std::string locale = preferred_locale.has_value()
+                           ? preferred_locale.value()
+                           : Config::GetDefaultPreferredLocale();
   if (tutorial_group_.has_value() && tutorial_group_->language == locale) {
     std::move(callback).Run(tutorial_group_->tutorials);
     return;
@@ -54,10 +59,10 @@ const std::vector<std::string>& TutorialManagerImpl::GetSupportedLanguages() {
   return supported_languages_;
 }
 
-std::string TutorialManagerImpl::GetPreferredLocale() {
-  return prefs_->HasPrefPath(kPreferredLocaleKey)
-             ? prefs_->GetString(kPreferredLocaleKey)
-             : Config::GetDefaultPreferredLocale();
+base::Optional<std::string> TutorialManagerImpl::GetPreferredLocale() {
+  if (prefs_->HasPrefPath(kPreferredLocaleKey))
+    return prefs_->GetString(kPreferredLocaleKey);
+  return base::nullopt;
 }
 
 void TutorialManagerImpl::SetPreferredLocale(const std::string& locale) {

chrome/browser/video_tutorials/internal/tutorial_manager_impl.h

@@ -30,7 +30,7 @@ class TutorialManagerImpl : public TutorialManager {
   // TutorialManager implementation.
   void GetTutorials(GetTutorialsCallback callback) override;
   const std::vector<std::string>& GetSupportedLanguages() override;
-  std::string GetPreferredLocale() override;
+  base::Optional<std::string> GetPreferredLocale() override;
   void SetPreferredLocale(const std::string& locale) override;
   void SaveGroups(std::unique_ptr<std::vector<TutorialGroup>> groups,
                   SuccessCallback callback) override;

chrome/browser/video_tutorials/internal/tutorial_service_impl.cc

@@ -89,7 +89,7 @@ const std::vector<std::string>& TutorialServiceImpl::GetSupportedLanguages() {
   return tutorial_manager_->GetSupportedLanguages();
 }
 
-std::string TutorialServiceImpl::GetPreferredLocale() {
+base::Optional<std::string> TutorialServiceImpl::GetPreferredLocale() {
   return tutorial_manager_->GetPreferredLocale();
 }
 

chrome/browser/video_tutorials/internal/tutorial_service_impl.h

@@ -26,7 +26,7 @@ class TutorialServiceImpl : public VideoTutorialService {
   void GetTutorial(FeatureType feature_type,
                    SingleItemCallback callback) override;
   const std::vector<std::string>& GetSupportedLanguages() override;
-  std::string GetPreferredLocale() override;
+  base::Optional<std::string> GetPreferredLocale() override;
   void SetPreferredLocale(const std::string& locale) override;
 
  private:

chrome/browser/video_tutorials/video_tutorial_service.h

@@ -35,8 +35,8 @@ class VideoTutorialService : public KeyedService,
   // Called to retrieve all the supported locales.
   virtual const std::vector<std::string>& GetSupportedLanguages() = 0;
 
-  // Called to retrieve the preferred locale.
-  virtual std::string GetPreferredLocale() = 0;
+  // Called to retrieve the preferred locale, if it is set by the user.
+  virtual base::Optional<std::string> GetPreferredLocale() = 0;
 
   // Called to set the preferred locale.
   virtual void SetPreferredLocale(const std::string& locale) = 0;