From c2449b8361d8ab382e69244399352948125c9dc8 Mon Sep 17 00:00:00 2001 From: Abodunrinwa Toki Date: Tue, 1 May 2018 21:36:48 +0100 Subject: [PATCH] Refresh TCM settings when they change Approach here is to register a content observer that invalidates the TC settings whenever updates to the settings happen. This CL also ensures that the TC is invalidated when a settings update happens. This is because the settings may change what TC the system returns. TextView's SelectionActionModeHelper has been updated to not cache the settings and get them directly from TCM (which caches the settings). NOTE that we expect TC settings to rarely change. Bug: 77539576 Test: bit FrameworksCoreTests:android.view.textclassifier.TextClassificationManagerTest Test: bit CtsViewTestCases:android.view.textclassifier.cts.TextClassificationManagerTest Test: bit CtsWidgetTestCases:android.widget.cts.TextViewTest Test: bit FrameworksCoreTests:android.widget.TextViewActivityTest Test: manual - Made changes to TC settings and observed logs / app behaviour Change-Id: I88bbb6f951708b17323fac1a72385fe808d270a5 --- .../textclassifier/TextClassificationManager.java | 98 +++++++++++++++++----- .../android/widget/SelectionActionModeHelper.java | 21 +++-- 2 files changed, 89 insertions(+), 30 deletions(-) diff --git a/core/java/android/view/textclassifier/TextClassificationManager.java b/core/java/android/view/textclassifier/TextClassificationManager.java index 1737861ee716..aee0aa719bc2 100644 --- a/core/java/android/view/textclassifier/TextClassificationManager.java +++ b/core/java/android/view/textclassifier/TextClassificationManager.java @@ -20,6 +20,7 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.SystemService; import android.content.Context; +import android.database.ContentObserver; import android.os.ServiceManager; import android.provider.Settings; import android.service.textclassifier.TextClassifierService; @@ -28,6 +29,8 @@ import android.view.textclassifier.TextClassifier.TextClassifierType; import com.android.internal.annotations.GuardedBy; import com.android.internal.util.Preconditions; +import java.lang.ref.WeakReference; + /** * Interface to the text classification service. */ @@ -42,23 +45,27 @@ public final class TextClassificationManager { classificationContext, getTextClassifier()); private final Context mContext; - private final TextClassificationConstants mSettings; + private final SettingsObserver mSettingsObserver; @GuardedBy("mLock") - private TextClassifier mTextClassifier; + @Nullable + private TextClassifier mCustomTextClassifier; @GuardedBy("mLock") + @Nullable private TextClassifier mLocalTextClassifier; @GuardedBy("mLock") + @Nullable private TextClassifier mSystemTextClassifier; @GuardedBy("mLock") private TextClassificationSessionFactory mSessionFactory; + @GuardedBy("mLock") + private TextClassificationConstants mSettings; /** @hide */ public TextClassificationManager(Context context) { mContext = Preconditions.checkNotNull(context); - mSettings = TextClassificationConstants.loadFromString(Settings.Global.getString( - context.getContentResolver(), Settings.Global.TEXT_CLASSIFIER_CONSTANTS)); mSessionFactory = mDefaultSessionFactory; + mSettingsObserver = new SettingsObserver(this); } /** @@ -71,14 +78,13 @@ public final class TextClassificationManager { @NonNull public TextClassifier getTextClassifier() { synchronized (mLock) { - if (mTextClassifier == null) { - if (isSystemTextClassifierEnabled()) { - mTextClassifier = getSystemTextClassifier(); - } else { - mTextClassifier = getLocalTextClassifier(); - } + if (mCustomTextClassifier != null) { + return mCustomTextClassifier; + } else if (isSystemTextClassifierEnabled()) { + return getSystemTextClassifier(); + } else { + return getLocalTextClassifier(); } - return mTextClassifier; } } @@ -89,7 +95,7 @@ public final class TextClassificationManager { */ public void setTextClassifier(@Nullable TextClassifier textClassifier) { synchronized (mLock) { - mTextClassifier = textClassifier; + mCustomTextClassifier = textClassifier; } } @@ -110,9 +116,15 @@ public final class TextClassificationManager { } } - /** @hide */ - public TextClassificationConstants getSettings() { - return mSettings; + private TextClassificationConstants getSettings() { + synchronized (mLock) { + if (mSettings == null) { + mSettings = TextClassificationConstants.loadFromString(Settings.Global.getString( + mContext.getApplicationContext().getContentResolver(), + Settings.Global.TEXT_CLASSIFIER_CONSTANTS)); + } + return mSettings; + } } /** @@ -170,11 +182,24 @@ public final class TextClassificationManager { } } + @Override + protected void finalize() throws Throwable { + try { + // Note that fields could be null if the constructor threw. + if (mContext != null && mSettingsObserver != null) { + mContext.getApplicationContext().getContentResolver() + .unregisterContentObserver(mSettingsObserver); + } + } finally { + super.finalize(); + } + } + private TextClassifier getSystemTextClassifier() { synchronized (mLock) { if (mSystemTextClassifier == null && isSystemTextClassifierEnabled()) { try { - mSystemTextClassifier = new SystemTextClassifier(mContext, mSettings); + mSystemTextClassifier = new SystemTextClassifier(mContext, getSettings()); Log.d(LOG_TAG, "Initialized SystemTextClassifier"); } catch (ServiceManager.ServiceNotFoundException e) { Log.e(LOG_TAG, "Could not initialize SystemTextClassifier", e); @@ -190,9 +215,9 @@ public final class TextClassificationManager { private TextClassifier getLocalTextClassifier() { synchronized (mLock) { if (mLocalTextClassifier == null) { - if (mSettings.isLocalTextClassifierEnabled()) { + if (getSettings().isLocalTextClassifierEnabled()) { mLocalTextClassifier = - new TextClassifierImpl(mContext, mSettings, TextClassifier.NO_OP); + new TextClassifierImpl(mContext, getSettings(), TextClassifier.NO_OP); } else { Log.d(LOG_TAG, "Local TextClassifier disabled"); mLocalTextClassifier = TextClassifier.NO_OP; @@ -203,20 +228,51 @@ public final class TextClassificationManager { } private boolean isSystemTextClassifierEnabled() { - return mSettings.isSystemTextClassifierEnabled() + return getSettings().isSystemTextClassifierEnabled() && TextClassifierService.getServiceComponentName(mContext) != null; } + private void invalidate() { + synchronized (mLock) { + mSettings = null; + mLocalTextClassifier = null; + mSystemTextClassifier = null; + } + } + /** @hide */ public static TextClassificationConstants getSettings(Context context) { Preconditions.checkNotNull(context); final TextClassificationManager tcm = context.getSystemService(TextClassificationManager.class); if (tcm != null) { - return tcm.mSettings; + return tcm.getSettings(); } else { return TextClassificationConstants.loadFromString(Settings.Global.getString( - context.getContentResolver(), Settings.Global.TEXT_CLASSIFIER_CONSTANTS)); + context.getApplicationContext().getContentResolver(), + Settings.Global.TEXT_CLASSIFIER_CONSTANTS)); + } + } + + private static final class SettingsObserver extends ContentObserver { + + private final WeakReference mTcm; + + SettingsObserver(TextClassificationManager tcm) { + super(null); + mTcm = new WeakReference<>(tcm); + tcm.mContext.getApplicationContext().getContentResolver().registerContentObserver( + Settings.Global.getUriFor(Settings.Global.TEXT_CLASSIFIER_CONSTANTS), + false /* notifyForDescendants */, + this); + } + + @Override + public void onChange(boolean selfChange) { + final TextClassificationManager tcm = mTcm.get(); + if (tcm != null) { + tcm.invalidate(); + } } } } diff --git a/core/java/android/widget/SelectionActionModeHelper.java b/core/java/android/widget/SelectionActionModeHelper.java index 1f2b90a11616..8f1f1ab2e5c0 100644 --- a/core/java/android/widget/SelectionActionModeHelper.java +++ b/core/java/android/widget/SelectionActionModeHelper.java @@ -70,7 +70,6 @@ public final class SelectionActionModeHelper { private final Editor mEditor; private final TextView mTextView; private final TextClassificationHelper mTextClassificationHelper; - private final TextClassificationConstants mTextClassificationSettings; @Nullable private TextClassification mTextClassification; private AsyncTask mTextClassificationAsyncTask; @@ -84,7 +83,6 @@ public final class SelectionActionModeHelper { SelectionActionModeHelper(@NonNull Editor editor) { mEditor = Preconditions.checkNotNull(editor); mTextView = mEditor.getTextView(); - mTextClassificationSettings = TextClassificationManager.getSettings(mTextView.getContext()); mTextClassificationHelper = new TextClassificationHelper( mTextView.getContext(), mTextView::getTextClassifier, @@ -92,7 +90,7 @@ public final class SelectionActionModeHelper { 0, 1, mTextView.getTextLocales()); mSelectionTracker = new SelectionTracker(mTextView); - if (mTextClassificationSettings.isSmartSelectionAnimationEnabled()) { + if (getTextClassificationSettings().isSmartSelectionAnimationEnabled()) { mSmartSelectSprite = new SmartSelectSprite(mTextView.getContext(), editor.getTextView().mHighlightColor, mTextView::invalidate); } else { @@ -105,7 +103,7 @@ public final class SelectionActionModeHelper { */ public void startSelectionActionModeAsync(boolean adjustSelection) { // Check if the smart selection should run for editable text. - adjustSelection &= mTextClassificationSettings.isSmartSelectionEnabled(); + adjustSelection &= getTextClassificationSettings().isSmartSelectionEnabled(); mSelectionTracker.onOriginalSelection( getText(mTextView), @@ -212,6 +210,10 @@ public final class SelectionActionModeHelper { return mSmartSelectSprite != null && mSmartSelectSprite.isAnimationActive(); } + private TextClassificationConstants getTextClassificationSettings() { + return TextClassificationManager.getSettings(mTextView.getContext()); + } + private void cancelAsyncTask() { if (mTextClassificationAsyncTask != null) { mTextClassificationAsyncTask.cancel(true); @@ -245,7 +247,7 @@ public final class SelectionActionModeHelper { if (result != null && text instanceof Spannable && (mTextView.isTextSelectable() || mTextView.isTextEditable())) { // Do not change the selection if TextClassifier should be dark launched. - if (!mTextClassificationSettings.isModelDarkLaunchEnabled()) { + if (!getTextClassificationSettings().isModelDarkLaunchEnabled()) { Selection.setSelection((Spannable) text, result.mStart, result.mEnd); mTextView.invalidate(); } @@ -906,7 +908,6 @@ public final class SelectionActionModeHelper { private static final int TRIM_DELTA = 120; // characters private final Context mContext; - private final boolean mDarkLaunchEnabled; private Supplier mTextClassifier; /** The original TextView text. **/ @@ -942,8 +943,6 @@ public final class SelectionActionModeHelper { CharSequence text, int selectionStart, int selectionEnd, LocaleList locales) { init(textClassifier, text, selectionStart, selectionEnd, locales); mContext = Preconditions.checkNotNull(context); - mDarkLaunchEnabled = TextClassificationManager.getSettings(mContext) - .isModelDarkLaunchEnabled(); } @UiThread @@ -982,7 +981,7 @@ public final class SelectionActionModeHelper { mTrimmedText, mRelativeStart, mRelativeEnd, mDefaultLocales); } // Do not classify new selection boundaries if TextClassifier should be dark launched. - if (!mDarkLaunchEnabled) { + if (!isDarkLaunchEnabled()) { mSelectionStart = Math.max(0, selection.getSelectionStartIndex() + mTrimStart); mSelectionEnd = Math.min( mText.length(), selection.getSelectionEndIndex() + mTrimStart); @@ -1010,6 +1009,10 @@ public final class SelectionActionModeHelper { } } + private boolean isDarkLaunchEnabled() { + return TextClassificationManager.getSettings(mContext).isModelDarkLaunchEnabled(); + } + private SelectionResult performClassification(@Nullable TextSelection selection) { if (!Objects.equals(mText, mLastClassificationText) || mSelectionStart != mLastClassificationSelectionStart -- 2.11.0