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

Commit 7f86847e authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Prevent unstateful edits, empty trimming, INSERT edge cases. 

Monkeys found some edge cases that could perform actions
when editor was in an invalid state.  Added validity checks
in about a dozen places to prevent.  Fixes http://b/2121368
and http://b/2115921

Hook up trimming of extra empty fields that users have
left blank, including dropping of entire RawContact if no
valid fields remain.  Also wrote a pile of unit tests to
verify behavior.  Fixes http://b/2112915

Don't CCO when inserting bundles that use CharSequence
instead of Strings.  Don't NPE when inserting bundles and
we encounter a DataKind that the chosen ContactsSource isn't
able to handle.  Fixes http://b/2105737 and http://b/2118580

Insert bundles for INSERT_AND_EDIT cases where user chose
to edit an existing contact.  Fixes http://b/2098856 and
http://b/1645578

Adopted new framework back key API from hackbod.
parent a62e6db0

src/com/android/contacts/model/ContactsSource.java

+5 −0
Original line number Diff line number Diff line
@@ -115,6 +115,11 @@ public class ContactsSource { 
        return mInflatedLevel >= inflateLevel;

    }



    /** @hide exposed for unit tests */

    public void setInflatedLevel(int inflateLevel) {

        mInflatedLevel = inflateLevel;

    }



    /**

     * Ensure that the constraint rules behind this {@link ContactsSource} have

     * been inflated. Because this may involve parsing meta-data from

src/com/android/contacts/model/EntityDelta.java

+6 −1
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@ 


package com.android.contacts.model;



import com.android.contacts.model.ContactsSource.DataKind;

import com.android.contacts.model.ContactsSource.EditField;

import com.google.android.collect.Lists;

import com.google.android.collect.Maps;

import com.google.android.collect.Sets;
@@ -31,6 +33,8 @@ import android.os.Parcelable; 
import android.provider.BaseColumns;

import android.provider.ContactsContract.Data;

import android.provider.ContactsContract.RawContacts;

import android.text.TextUtils;

import android.util.Log;

import android.view.View;



import java.util.ArrayList;
@@ -294,6 +298,7 @@ public class EntityDelta implements Parcelable { 
            // Assert version is consistent while persisting changes

            final Long beforeId = mValues.getId();

            final Long beforeVersion = mValues.getAsLong(RawContacts.VERSION);

            if (beforeId == null || beforeVersion == null) return;



            final ContentProviderOperation.Builder builder = ContentProviderOperation

                    .newAssertQuery(RawContacts.CONTENT_URI);
@@ -525,7 +530,7 @@ public class EntityDelta implements Parcelable { 


        public boolean isUpdate() {

            // When "after" has some changes, action is "update"

            return beforeExists() && (mAfter.size() > 0);

            return beforeExists() && (mAfter != null && mAfter.size() > 0);

        }



        public boolean isInsert() {

src/com/android/contacts/model/EntityModifier.java

+43 −7
Original line number Diff line number Diff line
@@ -28,6 +28,8 @@ import android.database.Cursor; 
import android.os.Bundle;

import android.provider.ContactsContract.Data;

import android.provider.ContactsContract.Intents;

import android.provider.ContactsContract.RawContacts;

import android.provider.ContactsContract.CommonDataKinds.BaseTypes;

import android.provider.ContactsContract.CommonDataKinds.Email;

import android.provider.ContactsContract.CommonDataKinds.Im;

import android.provider.ContactsContract.CommonDataKinds.Phone;
@@ -338,13 +340,30 @@ public class EntityModifier { 
        return child;

    }



    /**

     * Processing to trim any empty {@link ValuesDelta} and {@link EntityDelta}

     * from the given {@link EntitySet}, assuming the given {@link Sources}

     * dictates the structure for various fields. This method ignores rows not

     * described by the {@link ContactsSource}.

     */

    public static void trimEmpty(EntitySet set, Sources sources) {

        for (EntityDelta state : set) {

            final String accountType = state.getValues().getAsString(RawContacts.ACCOUNT_TYPE);

            final ContactsSource source = sources.getInflatedSource(accountType,

                    ContactsSource.LEVEL_MIMETYPES);

            trimEmpty(state, source);

        }

    }



    /**

     * Processing to trim any empty {@link ValuesDelta} rows from the given

     * {@link EntityDelta}, assuming the given {@link ContactsSource} dictates

     * the structure for various fields. This method ignores rows not described

     * by the {@link ContactsSource}.

     */

    public static void trimEmpty(ContactsSource source, EntityDelta state) {

    public static void trimEmpty(EntityDelta state, ContactsSource source) {

        boolean hasValues = false;



        // Walk through entries for each well-known kind

        for (DataKind kind : source.getSortedDataKinds()) {

            final String mimeType = kind.mimeType;
@@ -352,14 +371,27 @@ public class EntityModifier { 
            if (entries == null) continue;



            for (ValuesDelta entry : entries) {

                // Test and remove this row if empty

                // Skip any values that haven't been touched

                final boolean touched = entry.isInsert() || entry.isUpdate();

                if (touched && EntityModifier.isEmpty(entry, kind)) {

                if (!touched) {

                    hasValues = true;

                    continue;

                }



                // Test and remove this row if empty

                if (EntityModifier.isEmpty(entry, kind)) {

                    // TODO: remove this verbose logging

                    Log.w(TAG, "Trimming: " + entry.toString());

                    entry.markDeleted();

                } else {

                    hasValues = true;

                }

            }

        }



        if (!hasValues) {

            // Trim overall entity if no children exist

            state.markDeleted();

        }

    }
@@ -454,7 +486,10 @@ public class EntityModifier { 
     */

    public static void parseExtras(EntityDelta state, DataKind kind, Bundle extras,

            String typeExtra, String valueExtra, String valueColumn) {

        final String value = extras.getString(valueExtra);

        final CharSequence value = extras.getCharSequence(valueExtra);



        // Bail early if source doesn't handle this type

        if (kind == null) return;



        // Bail when can't insert type, or value missing

        final boolean canInsert = EntityModifier.canInsert(state, kind);
@@ -462,16 +497,17 @@ public class EntityModifier { 
        if (!validValue || !canInsert) return;



        // Find exact type, or otherwise best type

        final int typeValue = extras.getInt(typeExtra, Integer.MIN_VALUE);

        final int typeValue = extras.getInt(typeExtra, BaseTypes.TYPE_CUSTOM);

        final EditType editType = EntityModifier.getBestValidType(state, kind, true, typeValue);



        // Create data row and fill with value

        final ValuesDelta child = EntityModifier.insertChild(state, kind, editType);

        child.put(valueColumn, value);

        child.put(valueColumn, value.toString());



        if (editType != null && editType.customColumn != null) {

            // Write down label when custom type picked

            child.put(editType.customColumn, extras.getString(typeExtra));

            final CharSequence customType = extras.getCharSequence(typeExtra);

            child.put(editType.customColumn, customType.toString());

        }

    }

}

src/com/android/contacts/model/Sources.java

+8 −2
Original line number Diff line number Diff line
@@ -66,14 +66,20 @@ public class Sources { 
    }



    /**

     * Internal constructor that only performs initial parsing. Obtain a

     * {@link android.provider.ContactsContract.RawContacts#ACCOUNT_TYPE}.

     * Internal constructor that only performs initial parsing.

     */

    private Sources(Context context) {

        mContext = context;

        loadAccounts();

    }



    /** @hide exposed for unit tests */

    public Sources(ContactsSource... sources) {

        for (ContactsSource source : sources) {

            mSources.put(source.accountType, source);

        }

    }



    /**

     * Blocking call to load all {@link AuthenticatorDescription} known by the

     * {@link AccountManager} on the system.

src/com/android/contacts/ui/EditContactActivity.java

+63 −36
Original line number Diff line number Diff line
@@ -62,7 +62,6 @@ import android.provider.ContactsContract.CommonDataKinds.StructuredName; 
import android.provider.ContactsContract.Contacts.Data;

import android.util.Log;

import android.view.ContextThemeWrapper;

import android.view.KeyEvent;

import android.view.LayoutInflater;

import android.view.Menu;

import android.view.MenuInflater;
@@ -90,19 +89,9 @@ public final class EditContactActivity extends Activity implements View.OnClickL 
    /** The launch code when picking a photo and the raw data is returned */

    private static final int PHOTO_PICKED_WITH_DATA = 3021;



    private static final int TOKEN_ENTITY = 41;



    private static final String KEY_EDIT_STATE = "state";

    private static final String KEY_SELECTED_RAW_CONTACT = "selected";



//    private static final String KEY_SELECTED_TAB_ID = "tabId";

//    private static final String KEY_CONTACT_ID = "contactId";



//    private int mSelectedTab = -1;



//    private long mSelectedRawContactId = -1;

//    private long mContactId = -1;



    private String mQuerySelection;



    private ScrollingTabWidget mTabWidget;
@@ -188,6 +177,20 @@ public final class EditContactActivity extends Activity implements View.OnClickL 


            target.mQuerySelection = selection;

            target.mState = EntitySet.fromQuery(resolver, selection, null, null);



            // Handle any incoming values that should be inserted

            final Bundle extras = intent.getExtras();

            final boolean hasExtras = extras.size() > 0;

            final boolean hasState = target.mState.size() > 0;

            if (hasExtras && hasState) {

                // Find source defining the first RawContact found

                final EntityDelta state = target.mState.get(0);

                final String accountType = state.getValues().getAsString(RawContacts.ACCOUNT_TYPE);

                final ContactsSource source = sources.getInflatedSource(accountType,

                        ContactsSource.LEVEL_CONSTRAINTS);

                EntityModifier.parseExtras(context, source, state, extras);

            }



            return null;

        }
@@ -203,11 +206,11 @@ public final class EditContactActivity extends Activity implements View.OnClickL 


    @Override

    protected void onSaveInstanceState(Bundle outState) {

        if (hasValidState()) {

            // Store entities with modifications

            outState.putParcelable(KEY_EDIT_STATE, mState);

            outState.putLong(KEY_SELECTED_RAW_CONTACT, getSelectedRawContactId());

//        outState.putLong(KEY_SELECTED_TAB_ID, mSelectedRawContactId);

//        outState.putLong(KEY_CONTACT_ID, mContactId);

        }



        super.onSaveInstanceState(outState);

    }
@@ -217,16 +220,13 @@ public final class EditContactActivity extends Activity implements View.OnClickL 
        // Read modifications from instance

        mState = savedInstanceState.<EntitySet> getParcelable(KEY_EDIT_STATE);



//        mSelectedRawContactId = savedInstanceState.getLong(KEY_SELECTED_TAB_ID);

//        mContactId = savedInstanceState.getLong(KEY_CONTACT_ID);



        Log.d(TAG, "onrestoreinstancestate");



        bindTabs();

        bindHeader();



        if (hasValidState()) {

            final long selectedId = savedInstanceState.getLong(KEY_SELECTED_RAW_CONTACT);

            setSelectedRawContactId(selectedId);

        }



        // Restore selected tab and any focus

        super.onRestoreInstanceState(savedInstanceState);
@@ -248,6 +248,14 @@ public final class EditContactActivity extends Activity implements View.OnClickL 
        mTabWidget.setCurrentTab(index);

    }



    /**

     * Check if our internal {@link #mState} is valid, usually checked before

     * performing user actions.

     */

    protected boolean hasValidState() {

        return mState != null && mState.size() > 0;

    }







    /**
@@ -256,6 +264,8 @@ public final class EditContactActivity extends Activity implements View.OnClickL 
     * {@link RawContacts}.

     */

    protected void bindTabs() {

        if (!hasValidState()) return;



        final Sources sources = Sources.getInstance(this);

        int selectedTab = 0;
@@ -287,6 +297,8 @@ public final class EditContactActivity extends Activity implements View.OnClickL 
     * primary {@link Data} change.

     */

    protected void bindHeader() {

        if (!hasValidState()) return;



        // TODO: rebuild header widget based on internal entities



        // TODO: fill header bar with newly parsed data for speed
@@ -304,8 +316,8 @@ public final class EditContactActivity extends Activity implements View.OnClickL 


    /** {@inheritDoc} */

    public void onTabSelectionChanged(int tabIndex, boolean clicked) {

        boolean validTab = mState != null && tabIndex >= 0 && tabIndex < mState.size();

        if (!validTab) return;

        if (!hasValidState()) return;

        if (tabIndex < 0 || tabIndex >= mState.size()) return;



        // Find entity and source for selected tab

        final EntityDelta entity = mState.get(tabIndex);
@@ -321,11 +333,13 @@ public final class EditContactActivity extends Activity implements View.OnClickL 


    /** {@inheritDoc} */

    public void onDisplayNameLongClick(View view) {

        if (!hasValidState()) return;

        this.createNameDialog().show();

    }



    /** {@inheritDoc} */

    public void onPhotoLongClick(View view) {

        if (!hasValidState()) return;

        this.createPhotoDialog().show();

    }
@@ -347,12 +361,8 @@ public final class EditContactActivity extends Activity implements View.OnClickL 


    /** {@inheritDoc} */

    @Override

    public boolean onKeyDown(int keyCode, KeyEvent event) {

        switch (keyCode) {

            case KeyEvent.KEYCODE_BACK:

                return doSaveAction();

        }

        return super.onKeyDown(keyCode, event);

    public void onBackPressed() {

        doSaveAction();

    }



    /** {@inheritDoc} */
@@ -456,11 +466,18 @@ public final class EditContactActivity extends Activity implements View.OnClickL 
        /** {@inheritDoc} */

        @Override

        protected Integer doInBackground(EditContactActivity target, EntitySet... params) {

            final ContentResolver resolver = target.getContentResolver();

            final Context context = target;

            final ContentResolver resolver = context.getContentResolver();



            EntitySet state = params[0];



            // Trim any empty fields, and RawContacts, before persisting

            final Sources sources = Sources.getInstance(context);

            EntityModifier.trimEmpty(state, sources);



            // Attempt to persist changes

            int tries = 0;

            Integer result = RESULT_FAILURE;

            EntitySet state = params[0];

            while (tries < PERSIST_TRIES) {

                try {

                    // Build operations and try applying
@@ -515,8 +532,7 @@ public final class EditContactActivity extends Activity implements View.OnClickL 
     * finishes the activity.

     */

    private boolean doSaveAction() {

        // Bail early if nothing to save

        if (mState == null || mState.size() == 0) return true;

        if (!hasValidState()) return false;



        // Pass back last-selected contact

        final int selectedTab = mTabWidget.getCurrentTab();
@@ -558,6 +574,7 @@ public final class EditContactActivity extends Activity implements View.OnClickL 
     * {@link EntityDelta} under the currently edited {@link Contacts}.

     */

    private boolean doAddAction() {

        // Adding is okay when missing state

        new AddContactTask(this).execute();

        return true;

    }
@@ -567,6 +584,8 @@ public final class EditContactActivity extends Activity implements View.OnClickL 
     * user confirmation before continuing.

     */

    private boolean doDeleteAction() {

        if (!hasValidState()) return false;



        this.createDeleteDialog().show();

        return true;

    }
@@ -575,6 +594,8 @@ public final class EditContactActivity extends Activity implements View.OnClickL 
     * Pick a specific photo to be added under the currently selected tab.

     */

    private boolean doPickPhotoAction() {

        if (!hasValidState()) return false;



        try {

            // Launch picker to choose photo for selected contact

            final Intent intent = ContactsUtils.getPhotoPickIntent();
@@ -589,6 +610,8 @@ public final class EditContactActivity extends Activity implements View.OnClickL 
     * Clear any existing photo under the currently selected tab.

     */

    public boolean doRemovePhotoAction() {

        if (!hasValidState()) return false;



        // Remove photo from selected contact

        mEditor.setPhotoBitmap(null);

        return true;
@@ -601,6 +624,8 @@ public final class EditContactActivity extends Activity implements View.OnClickL 


    /** {@inheritDoc} */

    public void onRequest(int request) {

        if (!hasValidState()) return;



        switch (request) {

            case EditorListener.REQUEST_PICK_PHOTO: {

                doPickPhotoAction();
@@ -704,7 +729,7 @@ public final class EditContactActivity extends Activity implements View.OnClickL 
            final DialogInterface.OnCancelListener cancelListener = new DialogInterface.OnCancelListener() {

                public void onCancel(DialogInterface dialog) {

                    // If nothing remains, close activity

                    if (target.mState == null || target.mState.size() == 0) {

                    if (!target.hasValidState()) {

                        target.finish();

                    }

                }
@@ -739,6 +764,8 @@ public final class EditContactActivity extends Activity implements View.OnClickL 
                delta.markDeleted();



                // TODO: trigger task to update tabs (doesnt need to be background)

                bindTabs();

                bindHeader();

            }

        });

        builder.setNegativeButton(android.R.string.cancel, null);