Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a6d7b8c9 authored by Chaohui Wang's avatar Chaohui Wang
Browse files

Improve the robustness of updateNonIndexableKeys 

- Isolate preference controller crash in the same page
  Current if one search item throw exception, entire page's search will
  broken, can isolate issue by controller to improve.

- Expose preference controller crash in debuggable build
  Current the search index exception is silently ignored, expose the
  exception in debuggable builds to expose issue earlier.

- Change default to hide items instead of shown when crash

Fix: 352455031
Fix: 352455432
Fix: 352455540
Flag: EXEMPT bug fix
Test: manual - Settings Search
Change-Id: I2cdbd58543be8fe6617e42d9c02789791186f53b
parent 86e5406c

src/com/android/settings/search/BaseSearchIndexProvider.java

+40 −14
Original line number Diff line number Diff line
@@ -21,9 +21,11 @@ import static com.android.settings.core.PreferenceXmlParserUtils.METADATA_SEARCH 
import static com.android.settings.core.PreferenceXmlParserUtils.MetadataFlag.FLAG_INCLUDE_PREF_SCREEN;

import static com.android.settings.core.PreferenceXmlParserUtils.MetadataFlag.FLAG_NEED_KEY;

import static com.android.settings.core.PreferenceXmlParserUtils.MetadataFlag.FLAG_NEED_SEARCHABLE;

import static com.android.settings.search.SettingsSearchIndexablesProvider.SYSPROP_CRASH_ON_ERROR;



import android.annotation.XmlRes;

import android.content.Context;

import android.os.Build;

import android.os.Bundle;

import android.provider.SearchIndexableResource;

import android.util.Log;
@@ -131,25 +133,49 @@ public class BaseSearchIndexProvider implements Indexable.SearchIndexProvider { 
            return nonIndexableKeys;

        }

        nonIndexableKeys.addAll(getNonIndexableKeysFromXml(context, false /* suppressAllPage */));

        updateNonIndexableKeysFromControllers(context, nonIndexableKeys);

        return nonIndexableKeys;

    }



    private void updateNonIndexableKeysFromControllers(

            Context context, List<String> nonIndexableKeys) {

        final List<AbstractPreferenceController> controllers = getPreferenceControllers(context);

        if (controllers != null && !controllers.isEmpty()) {

        if (controllers != null) {

            for (AbstractPreferenceController controller : controllers) {

                if (controller instanceof PreferenceControllerMixin) {

                    ((PreferenceControllerMixin) controller)

                            .updateNonIndexableKeys(nonIndexableKeys);

                } else if (controller instanceof BasePreferenceController) {

                    ((BasePreferenceController) controller).updateNonIndexableKeys(

                            nonIndexableKeys);

                updateNonIndexableKeysFromController(nonIndexableKeys, controller);

            }

        }

    }



    private static void updateNonIndexableKeysFromController(

            List<String> nonIndexableKeys, AbstractPreferenceController controller) {

        try {

            if (controller instanceof PreferenceControllerMixin controllerMixin) {

                controllerMixin.updateNonIndexableKeys(nonIndexableKeys);

            } else if (controller instanceof BasePreferenceController basePreferenceController) {

                basePreferenceController.updateNonIndexableKeys(nonIndexableKeys);

            } else {

                Log.e(TAG, controller.getClass().getName()

                        + " must implement " + PreferenceControllerMixin.class.getName()

                        + " treating the key non-indexable");

                nonIndexableKeys.add(controller.getPreferenceKey());

            }

        } catch (Exception e) {

            String msg = "Error trying to get non-indexable keys from: " + controller;

            // Catch a generic crash. In the absence of the catch, the background thread will

            // silently fail anyway, so we aren't losing information by catching the exception.

            // We crash on debuggable build or when the system property exists, so that we can test

            // if crashes need to be fixed.

            // The gain is that if there is a crash in a specific controller, we don't lose all

            // non-indexable keys, but we can still find specific crashes in development.

            if (Build.IS_DEBUGGABLE || System.getProperty(SYSPROP_CRASH_ON_ERROR) != null) {

                throw new RuntimeException(msg, e);

            }

            Log.e(TAG, msg, e);

            // When there is an error, treat the key as non-indexable.

            nonIndexableKeys.add(controller.getPreferenceKey());

        }

    }

        return nonIndexableKeys;

    }



    public List<AbstractPreferenceController> getPreferenceControllers(Context context) {

        List<AbstractPreferenceController> controllersFromCode = new ArrayList<>();

src/com/android/settings/search/SettingsSearchIndexablesProvider.java

+8 −8
Original line number Diff line number Diff line
@@ -50,6 +50,7 @@ import android.content.Context; 
import android.database.Cursor;

import android.database.MatrixCursor;

import android.net.Uri;

import android.os.Build;

import android.provider.SearchIndexableResource;

import android.provider.SearchIndexablesContract;

import android.provider.SearchIndexablesProvider;
@@ -283,17 +284,16 @@ public class SettingsSearchIndexablesProvider extends SearchIndexablesProvider { 
            try {

                providerNonIndexableKeys = provider.getNonIndexableKeys(context);

            } catch (Exception e) {

                String msg = "Error trying to get non-indexable keys from: "

                        + bundle.getTargetClass().getName();

                // Catch a generic crash. In the absence of the catch, the background thread will

                // silently fail anyway, so we aren't losing information by catching the exception.

                // We crash when the system property exists so that we can test if crashes need to

                // be fixed.

                // The gain is that if there is a crash in a specific controller, we don't lose all

                // non-indexable keys, but we can still find specific crashes in development.

                if (System.getProperty(SYSPROP_CRASH_ON_ERROR) != null) {

                    throw new RuntimeException(e);

                }

                Log.e(TAG, "Error trying to get non-indexable keys from: "

                        + bundle.getTargetClass().getName(), e);

                // We crash on debuggable build or when the system property exists, so that we can

                // test if crashes need to be fixed.

                if (Build.IS_DEBUGGABLE || System.getProperty(SYSPROP_CRASH_ON_ERROR) != null) {

                    throw new RuntimeException(msg, e);

                }

                Log.e(TAG, msg, e);

                continue;

            }