Fix IndexOutOfBoundsException when darken web contents flag is enabled

All RadioButtonWithDescriptions are hosted under the same layout now.
Logic that injected the additional checkbox option now needs to account
for that change. Also leaving some explinations in there to make the
file a bit more descriptive.

Bug: 1020890
Change-Id: Id46e4d0ef56aee168d55f85882a4a100d872fde0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1898710
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714549}

chrome/android/java/src/org/chromium/chrome/browser/preferences/themes/RadioButtonGroupThemePreference.java

@@ -20,23 +20,28 @@ import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.night_mode.NightModeMetrics;
 import org.chromium.chrome.browser.preferences.themes.ThemePreferences.ThemeSetting;
 import org.chromium.chrome.browser.ui.widget.RadioButtonWithDescription;
+import org.chromium.chrome.browser.ui.widget.RadioButtonWithDescriptionLayout;
 
 import java.util.ArrayList;
 import java.util.Collections;
 
 /**
  * A radio button group Preference used for Themes. Currently, it has 3 options: System default,
- * Light, and Dark.
+ * Light, and Dark. When an additional flag, DARKEN_WEBSITES_CHECKBOX_IN_THEMES_SETTING, is active
+ * there is an option added underneath the currently selected preference to allow website contents
+ * to be darkened (active for System default and Dark).
  */
 public class RadioButtonGroupThemePreference
         extends Preference implements RadioGroup.OnCheckedChangeListener {
     private @ThemeSetting int mSetting;
+    private RadioButtonWithDescription mSettingRadioButton;
+    private RadioButtonWithDescriptionLayout mGroup;
     private ArrayList<RadioButtonWithDescription> mButtons;
+
+    // Additional view that darkens website contents.
+    private LinearLayout mCheckboxContainer;
     private boolean mDarkenWebsitesEnabled;
     private CheckBox mCheckBox;
-    private LinearLayout mLayoutContainer;
-    private LinearLayout mCheckboxContainer;
-    private RadioGroup mRadioGroup;
 
     public RadioButtonGroupThemePreference(Context context, AttributeSet attrs) {
         super(context, attrs);
@@ -61,10 +66,9 @@ public class RadioButtonGroupThemePreference
         super.onBindViewHolder(holder);
         mCheckboxContainer = (LinearLayout) holder.findViewById(R.id.checkbox_container);
         mCheckBox = (CheckBox) holder.findViewById(R.id.darken_websites);
-        mLayoutContainer = (LinearLayout) holder.itemView;
 
-        mRadioGroup = (RadioGroup) holder.findViewById(R.id.radio_button_layout);
-        mRadioGroup.setOnCheckedChangeListener(this);
+        mGroup = (RadioButtonWithDescriptionLayout) holder.findViewById(R.id.radio_button_layout);
+        mGroup.setOnCheckedChangeListener(this);
 
         mCheckboxContainer.setOnClickListener(x -> {
             mCheckBox.setChecked(!mCheckBox.isChecked());
@@ -86,20 +90,20 @@ public class RadioButtonGroupThemePreference
         mButtons.set(
                 ThemeSetting.DARK, (RadioButtonWithDescription) holder.findViewById(R.id.dark));
 
-        mButtons.get(mSetting).setChecked(true);
+        mSettingRadioButton = mButtons.get(mSetting);
+        mSettingRadioButton.setChecked(true);
         positionCheckbox();
     }
 
     /**
-     * Remove and insert the checkbox to the view, based on the value of mSetting
+     * Remove and insert the checkbox to the view, based on the current theme preference.
      */
     private void positionCheckbox() {
         if (ChromeFeatureList.isEnabled(
                     ChromeFeatureList.DARKEN_WEBSITES_CHECKBOX_IN_THEMES_SETTING)) {
             if (mSetting == ThemeSetting.SYSTEM_DEFAULT || mSetting == ThemeSetting.DARK) {
-                mLayoutContainer.removeView(mCheckboxContainer);
+                mGroup.attachAccessoryView(mCheckboxContainer, mSettingRadioButton);
                 mCheckboxContainer.setVisibility(View.VISIBLE);
-                mLayoutContainer.addView(mCheckboxContainer, mSetting + 1);
             } else {
                 mCheckboxContainer.setVisibility(View.GONE);
             }
@@ -111,9 +115,12 @@ public class RadioButtonGroupThemePreference
         for (int i = 0; i < ThemeSetting.NUM_ENTRIES; i++) {
             if (mButtons.get(i).isChecked()) {
                 mSetting = i;
+                mSettingRadioButton = mButtons.get(i);
                 break;
             }
         }
+        assert mSetting >= 0 && mSetting < ThemeSetting.NUM_ENTRIES : "No matching setting found.";
+
         positionCheckbox();
         callChangeListener(mSetting);
         NightModeMetrics.recordThemePreferencesChanged(mSetting);

chrome/browser/ui/android/widget/java/src/org/chromium/chrome/browser/ui/widget/RadioButtonWithDescription.java

@@ -17,7 +17,6 @@ import android.view.View.OnClickListener;
 import android.widget.RadioButton;
 import android.widget.RelativeLayout;
 import android.widget.TextView;
-
 import java.util.List;
 
 /**

chrome/browser/ui/android/widget/java/src/org/chromium/chrome/browser/ui/widget/RadioButtonWithDescriptionLayout.java

@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.ui.widget;
 import android.content.Context;
 import android.util.AttributeSet;
 import android.view.View;
+import android.view.ViewGroup;
 import android.widget.RadioGroup;
 
 import androidx.annotation.Nullable;
@@ -16,11 +17,13 @@ import java.util.List;
 
 /**
  * Manages a group of exclusive RadioButtonWithDescriptions, automatically inserting a margin in
- * between the rows to prevent them from squishing together.
+ * between the rows to prevent them from squishing together. Has the option to set an accessory view
+ * on any given RadioButtonWithDescription. Only one accessory view per layout is supported.
  *
  * -------------------------------------------------
  * | O | MESSAGE #1                                |
  *        description_1                            |
+ *        [optional] accessory view                |
  * | O | MESSAGE #N                                |
  *        description_n                            |
  * -------------------------------------------------
@@ -63,6 +66,7 @@ public final class RadioButtonWithDescriptionLayout
     private final int mMarginBetweenRows;
     private final List<RadioButtonWithDescription> mRadioButtonsWithDescriptions;
     private OnCheckedChangeListener mOnCheckedChangeListener;
+    private View mAccessoryView;
 
     public RadioButtonWithDescriptionLayout(Context context) {
         this(context, null);
@@ -130,6 +134,31 @@ public final class RadioButtonWithDescriptionLayout
         updateMargins();
     }
 
+    private View removeAttachedAccessoryView(View view) {
+        // Remove the view from it's parent if it has one.
+        if (view.getParent() != null) {
+            ViewGroup previousGroup = (ViewGroup) view.getParent();
+            previousGroup.removeView(view);
+        }
+        return view;
+    }
+
+    /**
+     * Attach the given accessory view to the given RadioButtonWithDescription. The attachmentPoint
+     * must be a direct child of this.
+     *
+     * @param accessoryView The accessory view to be attached.
+     * @param attachmentPoint The RadioButtonWithDescription that the accessory view will be
+     *                        attached to.
+     */
+    public void attachAccessoryView(
+            View accessoryView, RadioButtonWithDescription attachmentPoint) {
+        removeAttachedAccessoryView(accessoryView);
+        int attachmentPointIndex = indexOfChild(attachmentPoint);
+        assert attachmentPointIndex >= 0 : "attachmentPoint view must a child of layout.";
+        addView(accessoryView, attachmentPointIndex + 1);
+    }
+
     /** Sets margins between each of the radio buttons. */
     private void updateMargins() {
         int childCount = getChildCount();

chrome/browser/ui/android/widget/java/src/org/chromium/chrome/browser/ui/widget/RadioButtonWithDescriptionLayoutTest.java

@@ -11,6 +11,7 @@ import android.support.test.filters.SmallTest;
 import android.support.test.rule.UiThreadTestRule;
 import android.view.View;
 import android.view.ViewGroup.MarginLayoutParams;
+import android.widget.TextView;
 
 import org.junit.Assert;
 import org.junit.Before;
@@ -187,4 +188,51 @@ public class RadioButtonWithDescriptionLayoutTest {
             Assert.assertEquals(i == 1, child.isChecked());
         }
     }
+
+    @Test
+    @SmallTest
+    @UiThreadTest
+    public void testAccessoryViewAdded() {
+        final RadioButtonWithDescriptionLayout layout =
+                new RadioButtonWithDescriptionLayout(mContext);
+
+        List<RadioButtonWithDescriptionLayout.Option> options = new ArrayList<>();
+        options.add(new RadioButtonWithDescriptionLayout.Option("a", "a_desc", null));
+        options.add(new RadioButtonWithDescriptionLayout.Option("b", "b_desc", null));
+        options.add(new RadioButtonWithDescriptionLayout.Option("c", "c_desc", null));
+        layout.addOptions(options);
+
+        RadioButtonWithDescription firstButton = (RadioButtonWithDescription) layout.getChildAt(0);
+        final TextView accessoryTextView = new TextView(mContext);
+        layout.attachAccessoryView(accessoryTextView, firstButton);
+        Assert.assertEquals(
+                "The accessory view should be right after the position of it's attachment host.",
+                accessoryTextView, layout.getChildAt(1));
+    }
+
+    @Test
+    @SmallTest
+    @UiThreadTest
+    public void testAccessoryViewAddedThenReadded() {
+        final RadioButtonWithDescriptionLayout layout =
+                new RadioButtonWithDescriptionLayout(mContext);
+
+        List<RadioButtonWithDescriptionLayout.Option> options = new ArrayList<>();
+        options.add(new RadioButtonWithDescriptionLayout.Option("a", "a_desc", null));
+        options.add(new RadioButtonWithDescriptionLayout.Option("b", "b_desc", null));
+        options.add(new RadioButtonWithDescriptionLayout.Option("c", "c_desc", null));
+        layout.addOptions(options);
+
+        RadioButtonWithDescription firstButton = (RadioButtonWithDescription) layout.getChildAt(0);
+        RadioButtonWithDescription lastButton =
+                (RadioButtonWithDescription) layout.getChildAt(layout.getChildCount() - 1);
+        final TextView accessoryTextView = new TextView(mContext);
+        layout.attachAccessoryView(accessoryTextView, firstButton);
+        layout.attachAccessoryView(accessoryTextView, lastButton);
+        Assert.assertNotEquals(
+                "The accessory view shouldn't be in the first position it was inserted at.",
+                accessoryTextView, layout.getChildAt(1));
+        Assert.assertEquals("The accessory view should be at the new position it was placed at.",
+                accessoryTextView, layout.getChildAt(layout.getChildCount() - 1));
+    }
 }