Skip to content

Commit

Permalink
[Merge][AppLanguagePrompt] Add flag to not show prompt if ULP match
Browse files Browse the repository at this point in the history
This CL adds a flag that does not show the AppLanguagePrompt if the
Chrome UI base language matches they top ULP base language.  The
AppLanguagePrompt must be enabled for this flag to take effect.

If the UI language and top ULP language do not match or there are no
ULP languages then the previous behavior is used to determine if the
prompt should be shown.

This CL introduces the Language Bridge native code to pass the ULP
languages back to Java from the ULPLanguageModel. This required moving
JavaLanguageBridgeGetULPLanguagesWrapper from
language_model_manager_factory.cc to language_bridged.cc so the JNI file
was only imported in once location.

Note: this was not a clean cherry-pick. https://crrev.com/c/3601371
conflicted with this CL. See the the diff from PS1 -> PS2 for what was
edited.  The diff of base -> PS2 is correct.

(cherry picked from commit cf57677)

Bug: 1315618
Change-Id: Ie2d29e8daa8b193fd9b5bfb572d61184ca79ad8c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3600929
Reviewed-by: Josh Simmons <jds@google.com>
Commit-Queue: Trevor Perrier <perrier@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#995390}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3605849
Cr-Commit-Position: refs/branch-heads/5005@{#149}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}
  • Loading branch information
Trevor Perrier authored and Chromium LUCI CQ committed Apr 25, 2022
1 parent f84d145 commit ee38cd4
Show file tree
Hide file tree
Showing 13 changed files with 224 additions and 26 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/BUILD.gn
Expand Up @@ -3155,6 +3155,8 @@ static_library("browser") {
"installable/installed_webapp_provider.cc",
"installable/installed_webapp_provider.h",
"installable/quality_enforcer.cc",
"language/android/language_bridge.cc",
"language/android/language_bridge.h",
"lens/android/lens_policy_utils.cc",
"lens/android/lens_prefs.cc",
"lens/android/lens_prefs.h",
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/flags/android/chrome_feature_list.cc
Expand Up @@ -312,6 +312,7 @@ const base::Feature* const kFeaturesExposedToJava[] = {
&paint_preview::kPaintPreviewDemo,
&paint_preview::kPaintPreviewShowOnStartup,
&language::kAppLanguagePrompt,
&language::kAppLanguagePromptULP,
&language::kDetailedLanguageSettings,
&language::kExplicitLanguageAsk,
&language::kForceAppLanguagePrompt,
Expand Down
Expand Up @@ -220,6 +220,7 @@ public static boolean getFieldTrialParamByFeatureAsBoolean(
"AndroidSearchEngineChoiceNotification";
public static final String ANONYMOUS_UPDATE_CHECKS = "AnonymousUpdateChecks";
public static final String APP_LANGUAGE_PROMPT = "AppLanguagePrompt";
public static final String APP_LANGUAGE_PROMPT_ULP = "AppLanguagePromptULP";
public static final String ASSISTANT_CONSENT_MODAL = "AssistantConsentModal";
public static final String ASSISTANT_CONSENT_SIMPLIFIED_TEXT = "AssistantConsentSimplifiedText";
public static final String ASSISTANT_CONSENT_V2 = "AssistantConsentV2";
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/language/android/BUILD.gn
Expand Up @@ -116,7 +116,9 @@ android_library("javatests") {
sources = [
"java/src/org/chromium/chrome/browser/language/AppLanguagePromoDialogTest.java",
"java/src/org/chromium/chrome/browser/language/AppLocaleUtilsTest.java",
"java/src/org/chromium/chrome/browser/language/FakeLanguageBridgeJni.java",
"java/src/org/chromium/chrome/browser/language/GlobalAppLocaleControllerTest.java",
"java/src/org/chromium/chrome/browser/language/LanguageBridgeTest.java",
"java/src/org/chromium/chrome/browser/language/settings/LanguageSettingsTest.java",
"java/src/org/chromium/chrome/browser/language/settings/LanguagesManagerTest.java",
"java/src/org/chromium/chrome/browser/translate/FakeTranslateBridgeJni.java",
Expand Down
Expand Up @@ -519,9 +519,14 @@ public static boolean maybeShowPrompt(Activity activity,
private static boolean shouldShowPrompt() {
// Skip feature and preference checks if forced on for testing.
if (!ChromeFeatureList.isEnabled(ChromeFeatureList.FORCE_APP_LANGUAGE_PROMPT)) {
// Don't show the prompt if not enabled.
// Don't show if not enabled.
if (!ChromeFeatureList.isEnabled(ChromeFeatureList.APP_LANGUAGE_PROMPT)) return false;
// Don't show the prompt if it has already been shown.
// Don't show if ULP match is enabled and the UI language matches the top ULP language.
if (ChromeFeatureList.isEnabled(ChromeFeatureList.APP_LANGUAGE_PROMPT_ULP)
&& LanguageBridge.isTopULPBaseLanguage(Locale.getDefault().toLanguageTag())) {
return false;
}
// Don't show if it has already been shown.
if (TranslateBridge.getAppLanguagePromptShown()) return false;
}

Expand Down
@@ -0,0 +1,29 @@
// Copyright 2022 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.

package org.chromium.chrome.browser.language;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.LinkedHashSet;

/**
* Fake implementation of LanguageBridge native methods used for testing.
*/
public class FakeLanguageBridgeJni implements LanguageBridge.Natives {
private ArrayList<String> mULPLanguages;

public FakeLanguageBridgeJni() {
mULPLanguages = new ArrayList<String>();
}

public void setULPLanguages(String[] languageCodes) {
mULPLanguages = new ArrayList<>(Arrays.asList(languageCodes));
}

@Override
public void getULPModelLanguages(LinkedHashSet<String> set) {
set.addAll(mULPLanguages);
}
}
Expand Up @@ -6,17 +6,53 @@

import android.text.TextUtils;

import org.chromium.base.LocaleUtils;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.components.language.LanguageProfileController;
import org.chromium.components.language.LanguageProfileDelegateImpl;

import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;

/**
* Bridge class for native code to access ULP data for a profile.
* @param accountName Account name for current profile for querying ULP.
*/
public class LanguageBridge {
/**
* Returns true if the language tag matches the top ULP language. Only
* language bases are compared (e.g. en-US = en-GB).
* @param language String of language tag to check.
* @return True if the base language tag matches the top language.
*/
public static boolean isTopULPBaseLanguage(String language) {
LinkedHashSet<String> ulpLanguages = getULPModelLanguages();

Iterator<String> ulpIterator = ulpLanguages.iterator();
if (!ulpIterator.hasNext()) return false;
return LocaleUtils.isBaseLanguageEqual(language, ulpIterator.next());
}

/**
* @return The ordered set of ULP languages saved in the ULP Language Model
*/
public static LinkedHashSet<String> getULPModelLanguages() {
LinkedHashSet<String> set = new LinkedHashSet<>();
LanguageBridgeJni.get().getULPModelLanguages(set);
return set;
}

@CalledByNative
private static void copyStringArrayToCollection(Collection<String> c, String[] a) {
c.addAll(Arrays.asList(a));
}

/**
* Blocking call used by native ULPLanguageModel to get device ULP languages.
*/
@CalledByNative
public static String[] getULPLanguages(String accountName) {
LanguageProfileDelegateImpl delegate = new LanguageProfileDelegateImpl();
Expand All @@ -28,4 +64,9 @@ public static String[] getULPLanguages(String accountName) {
languages_array = languages_list.toArray(languages_array);
return languages_array;
}

@NativeMethods
interface Natives {
void getULPModelLanguages(LinkedHashSet<String> set);
}
}
@@ -0,0 +1,54 @@
// Copyright 2022 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.

package org.chromium.chrome.browser.language;

import androidx.test.filters.SmallTest;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.MockitoAnnotations;

import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.JniMocker;
import org.chromium.chrome.browser.flags.ChromeSwitches;
// import org.chromium.browser.language.FakeLanguageBridgeJni;
// import org.chromium.browser.language.LanguageBridge;
// import org.chromium.browser.language.LanguageBridgeJni;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;

/**
* Tests for {@link LanguageBridge} which gets language lists from native
*/
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
public class LanguageBridgeTest {
@Rule
public JniMocker mJniMocker = new JniMocker();

private FakeLanguageBridgeJni mFakeLanguageBridge;

@Before
public void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
// Setup fake language bridge JNI interface
mFakeLanguageBridge = new FakeLanguageBridgeJni();
String[] ulpLanguages = {"pt-BR", "en-US"};
mFakeLanguageBridge.setULPLanguages(ulpLanguages);
mJniMocker.mock(LanguageBridgeJni.TEST_HOOKS, mFakeLanguageBridge);
}

@Test
@SmallTest
public void testIsTopULPBaseLanguage() {
Assert.assertTrue(LanguageBridge.isTopULPBaseLanguage("pt"));
Assert.assertTrue(LanguageBridge.isTopULPBaseLanguage("pt-PT"));
Assert.assertTrue(LanguageBridge.isTopULPBaseLanguage("pt-BR"));
Assert.assertFalse(LanguageBridge.isTopULPBaseLanguage("en"));
Assert.assertFalse(LanguageBridge.isTopULPBaseLanguage("en-US"));
}
}
58 changes: 58 additions & 0 deletions chrome/browser/language/android/language_bridge.cc
@@ -0,0 +1,58 @@
// Copyright 2022 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.

#include "chrome/browser/language/android/language_bridge.h"

#include "base/android/jni_array.h"
#include "base/android/jni_string.h"
#include "chrome/browser/language/android/jni_headers/LanguageBridge_jni.h"
#include "chrome/browser/language/language_model_manager_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "components/language/core/browser/language_model.h"
#include "components/language/core/browser/language_model_manager.h"

namespace language {
std::vector<std::string> LanguageBridge::GetULPLanguages(
std::string account_name) {
JNIEnv* env = base::android::AttachCurrentThread();
base::android::ScopedJavaLocalRef<jstring> account_name_java =
base::android::ConvertUTF8ToJavaString(env, account_name);
base::android::ScopedJavaLocalRef<jobjectArray> languages_java =
Java_LanguageBridge_getULPLanguages(env, account_name_java);

const int num_langs = (*env).GetArrayLength(languages_java.obj());
std::vector<std::string> languages;
for (int i = 0; i < num_langs; i++) {
jstring language_name_java =
(jstring)(*env).GetObjectArrayElement(languages_java.obj(), i);
languages.emplace_back(
base::android::ConvertJavaStringToUTF8(env, language_name_java));
}

return languages;
}
} // namespace language

static void JNI_LanguageBridge_GetULPModelLanguages(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& set) {
Profile* profile = ProfileManager::GetActiveUserProfile();
language::LanguageModel* language_model =
LanguageModelManagerFactory::GetForBrowserContext(profile)
->GetLanguageModel(language::LanguageModelManager::ModelType::ULP);
if (!language_model)
return;

std::vector<language::LanguageModel::LanguageDetails> languageDetails =
language_model->GetLanguages();
DCHECK(!languageDetails.empty());

std::vector<std::string> languages;
for (const auto& details : languageDetails) {
languages.push_back(details.lang_code);
}
Java_LanguageBridge_copyStringArrayToCollection(
env, set, base::android::ToJavaArrayOfStrings(env, languages));
}
20 changes: 20 additions & 0 deletions chrome/browser/language/android/language_bridge.h
@@ -0,0 +1,20 @@
// Copyright 2022 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 CHROME_BROWSER_LANGUAGE_ANDROID_LANGUAGE_BRIDGE_H_
#define CHROME_BROWSER_LANGUAGE_ANDROID_LANGUAGE_BRIDGE_H_

#include <string>
#include <vector>

namespace language {
class LanguageBridge {
public:
// Makes a blocking call to get ULP languages for |account_name|
static std::vector<std::string> GetULPLanguages(std::string account_name);
};

} // namespace language

#endif // CHROME_BROWSER_LANGUAGE_ANDROID_LANGUAGE_BRIDGE_H_
26 changes: 3 additions & 23 deletions chrome/browser/language/language_model_manager_factory.cc
Expand Up @@ -32,7 +32,7 @@
#if BUILDFLAG(IS_ANDROID)
#include "base/android/jni_string.h"
#include "base/android/scoped_java_ref.h"
#include "chrome/browser/language/android/jni_headers/LanguageBridge_jni.h"
#include "chrome/browser/language/android/language_bridge.h"
#endif

namespace {
Expand Down Expand Up @@ -72,26 +72,6 @@ void RecordULPInitMetrics(Profile* profile,
ulp_languages));
}

std::vector<std::string> JavaLanguageBridgeGetULPLanguagesWrapper(
std::string account_name) {
JNIEnv* env = base::android::AttachCurrentThread();
base::android::ScopedJavaLocalRef<jstring> account_name_java =
base::android::ConvertUTF8ToJavaString(env, account_name);
base::android::ScopedJavaLocalRef<jobjectArray> languages_java =
Java_LanguageBridge_getULPLanguages(env, account_name_java);

const int num_langs = (*env).GetArrayLength(languages_java.obj());
std::vector<std::string> languages;
for (int i = 0; i < num_langs; i++) {
jstring language_name_java =
(jstring)(*env).GetObjectArrayElement(languages_java.obj(), i);
languages.emplace_back(
base::android::ConvertJavaStringToUTF8(env, language_name_java));
}

return languages;
}

void CreateAndAddULPLanguageModel(Profile* profile,
std::vector<std::string> languages) {
RecordULPInitMetrics(profile, languages);
Expand Down Expand Up @@ -137,8 +117,8 @@ void PrepareLanguageModels(Profile* const profile,
if (base::FeatureList::IsEnabled(language::kUseULPLanguagesInChrome)) {
base::ThreadPool::PostTaskAndReplyWithResult(
FROM_HERE, {base::MayBlock()},
base::BindOnce(&JavaLanguageBridgeGetULPLanguagesWrapper,
profile->GetProfileUserName()),
base::BindOnce(&language::LanguageBridge::GetULPLanguages,
profile->GetProfileUserName()),
base::BindOnce(&CreateAndAddULPLanguageModel, profile));
}
#endif
Expand Down
2 changes: 2 additions & 0 deletions components/language/core/common/language_experiments.cc
Expand Up @@ -20,6 +20,8 @@ const base::Feature kExplicitLanguageAsk{"ExplicitLanguageAsk",
base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kAppLanguagePrompt{"AppLanguagePrompt",
base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kAppLanguagePromptULP{"AppLanguagePromptULP",
base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kForceAppLanguagePrompt{"ForceAppLanguagePrompt",
base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kNotifySyncOnLanguageDetermined{
Expand Down
3 changes: 3 additions & 0 deletions components/language/core/common/language_experiments.h
Expand Up @@ -15,6 +15,9 @@ extern const base::Feature kExplicitLanguageAsk;
// The feature that enables a second run prompt to select the app UI language on
// Android.
extern const base::Feature kAppLanguagePrompt;
// When enabled does not show the AppLanguagePrompt to users whose base UI
// language is their top ULP language.
extern const base::Feature kAppLanguagePromptULP;
// This feature forces the app UI prompt even if it has already been shown.
extern const base::Feature kForceAppLanguagePrompt;

Expand Down

0 comments on commit ee38cd4

Please sign in to comment.