Commit 1d2b3827 authored by Christian Dullweber's avatar Christian Dullweber Committed by Commit Bot

Fix manage storage crash

ManageSpaceActivity would crash when the manage storage button is
clicked because it tried to create the wrong Fragment.
This is fixed and a test is added that clicks the button to verify that
it works.
Asserts are changed to exceptions to get more meaningful errors if there
are more false usages.

Bug: 1125062
Change-Id: Ie78af6d04c91d4b85b21b04b4f98605e0c4c46a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2395498Reviewed-by: default avatarKamila Hasanbega <hkamila@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804880}
parent 01e98276
...@@ -41,6 +41,7 @@ import org.chromium.chrome.browser.profiles.Profile; ...@@ -41,6 +41,7 @@ import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.searchwidget.SearchWidgetProvider; import org.chromium.chrome.browser.searchwidget.SearchWidgetProvider;
import org.chromium.chrome.browser.settings.SettingsLauncher; import org.chromium.chrome.browser.settings.SettingsLauncher;
import org.chromium.chrome.browser.settings.SettingsLauncherImpl; import org.chromium.chrome.browser.settings.SettingsLauncherImpl;
import org.chromium.components.browser_ui.site_settings.AllSiteSettings;
import org.chromium.components.browser_ui.site_settings.SingleCategorySettings; import org.chromium.components.browser_ui.site_settings.SingleCategorySettings;
import org.chromium.components.browser_ui.site_settings.SiteSettingsCategory; import org.chromium.components.browser_ui.site_settings.SiteSettingsCategory;
import org.chromium.components.browser_ui.site_settings.Website; import org.chromium.components.browser_ui.site_settings.Website;
...@@ -242,8 +243,7 @@ public class ManageSpaceActivity extends AppCompatActivity implements View.OnCli ...@@ -242,8 +243,7 @@ public class ManageSpaceActivity extends AppCompatActivity implements View.OnCli
RecordHistogram.recordEnumeratedHistogram( RecordHistogram.recordEnumeratedHistogram(
"Android.ManageSpace.ActionTaken", OPTION_MANAGE_STORAGE, OPTION_MAX); "Android.ManageSpace.ActionTaken", OPTION_MANAGE_STORAGE, OPTION_MAX);
SettingsLauncher settingsLauncher = new SettingsLauncherImpl(); SettingsLauncher settingsLauncher = new SettingsLauncherImpl();
settingsLauncher.launchSettingsActivity( settingsLauncher.launchSettingsActivity(this, AllSiteSettings.class, initialArguments);
this, SingleCategorySettings.class, initialArguments);
} else if (view == mClearAllDataButton) { } else if (view == mClearAllDataButton) {
final ActivityManager activityManager = final ActivityManager activityManager =
(ActivityManager) getSystemService(Context.ACTIVITY_SERVICE); (ActivityManager) getSystemService(Context.ACTIVITY_SERVICE);
......
...@@ -4,10 +4,18 @@ ...@@ -4,10 +4,18 @@
package org.chromium.chrome.browser.site_settings; package org.chromium.chrome.browser.site_settings;
import static androidx.test.espresso.Espresso.onView;
import static androidx.test.espresso.action.ViewActions.click;
import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.withId;
import static androidx.test.espresso.matcher.ViewMatchers.withText;
import android.content.Intent; import android.content.Intent;
import android.support.test.InstrumentationRegistry; import android.support.test.InstrumentationRegistry;
import androidx.appcompat.app.AlertDialog; import androidx.appcompat.app.AlertDialog;
import androidx.test.espresso.Espresso;
import androidx.test.espresso.assertion.ViewAssertions;
import androidx.test.filters.MediumTest; import androidx.test.filters.MediumTest;
import androidx.test.filters.SmallTest; import androidx.test.filters.SmallTest;
...@@ -21,6 +29,7 @@ import org.junit.runner.RunWith; ...@@ -21,6 +29,7 @@ import org.junit.runner.RunWith;
import org.chromium.base.test.util.CommandLineFlags; import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisabledTest; import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.app.ChromeActivity; import org.chromium.chrome.browser.app.ChromeActivity;
import org.chromium.chrome.browser.browsing_data.BrowsingDataBridge; import org.chromium.chrome.browser.browsing_data.BrowsingDataBridge;
import org.chromium.chrome.browser.flags.ChromeSwitches; import org.chromium.chrome.browser.flags.ChromeSwitches;
...@@ -149,6 +158,17 @@ public class ManageSpaceActivityTest { ...@@ -149,6 +158,17 @@ public class ManageSpaceActivityTest {
manageSpaceActivity.finish(); manageSpaceActivity.finish();
} }
@Test
@MediumTest
@Feature({"SiteEngagement"})
public void testManageSiteStorage() {
ManageSpaceActivity manageSpaceActivity = startManageSpaceActivity();
waitForClearButtonEnabled(manageSpaceActivity);
onView(withId(R.id.manage_site_data_storage)).perform(click());
Espresso.onView(withText("Data stored")).check(ViewAssertions.matches(isDisplayed()));
manageSpaceActivity.finish();
}
// TODO(dmurph): Test the other buttons. One should go to the site storage list, and the other // TODO(dmurph): Test the other buttons. One should go to the site storage list, and the other
// should reset all app data. // should reset all app data.
} }
...@@ -115,8 +115,10 @@ public class AllSiteSettings extends SiteSettingsPreferenceFragment ...@@ -115,8 +115,10 @@ public class AllSiteSettings extends SiteSettingsPreferenceFragment
mCategory = SiteSettingsCategory.createFromType( mCategory = SiteSettingsCategory.createFromType(
browserContextHandle, SiteSettingsCategory.Type.ALL_SITES); browserContextHandle, SiteSettingsCategory.Type.ALL_SITES);
} }
assert (mCategory.showSites(SiteSettingsCategory.Type.ALL_SITES) if (!(mCategory.showSites(SiteSettingsCategory.Type.ALL_SITES)
|| mCategory.showSites(SiteSettingsCategory.Type.USE_STORAGE)); || mCategory.showSites(SiteSettingsCategory.Type.USE_STORAGE))) {
throw new IllegalArgumentException("Use SingleCategorySettings instead.");
};
ViewGroup view = (ViewGroup) super.onCreateView(inflater, container, savedInstanceState); ViewGroup view = (ViewGroup) super.onCreateView(inflater, container, savedInstanceState);
......
...@@ -23,7 +23,6 @@ import android.view.MenuInflater; ...@@ -23,7 +23,6 @@ import android.view.MenuInflater;
import android.view.MenuItem; import android.view.MenuItem;
import android.view.View; import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import android.widget.TextView;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import androidx.preference.Preference; import androidx.preference.Preference;
...@@ -88,8 +87,6 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment ...@@ -88,8 +87,6 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment
// The list that contains preferences. // The list that contains preferences.
private RecyclerView mListView; private RecyclerView mListView;
// The view to show when the list is empty.
private TextView mEmptyView;
// The item for searching the list of items. // The item for searching the list of items.
private MenuItem mSearchItem; private MenuItem mSearchItem;
// The Site Settings Category we are showing. // The Site Settings Category we are showing.
...@@ -152,10 +149,6 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment ...@@ -152,10 +149,6 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment
int chooserDataType = mCategory.getObjectChooserDataType(); int chooserDataType = mCategory.getObjectChooserDataType();
boolean hasEntries = boolean hasEntries =
chooserDataType == -1 ? addWebsites(sites) : addChosenObjects(sites); chooserDataType == -1 ? addWebsites(sites) : addChosenObjects(sites);
if (mEmptyView == null) return;
mEmptyView.setVisibility(hasEntries ? View.GONE : View.VISIBLE);
} }
} }
...@@ -305,8 +298,10 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment ...@@ -305,8 +298,10 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment
browserContextHandle, getArguments().getString(EXTRA_CATEGORY, "")); browserContextHandle, getArguments().getString(EXTRA_CATEGORY, ""));
} }
assert !(mCategory.showSites(SiteSettingsCategory.Type.ALL_SITES) if (mCategory.showSites(SiteSettingsCategory.Type.ALL_SITES)
|| mCategory.showSites(SiteSettingsCategory.Type.USE_STORAGE)); || mCategory.showSites(SiteSettingsCategory.Type.USE_STORAGE)) {
throw new IllegalArgumentException("Use AllSiteSettings instead.");
}
int contentType = mCategory.getContentSettingsType(); int contentType = mCategory.getContentSettingsType();
mRequiresTriStateSetting = mRequiresTriStateSetting =
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment