Skip to content

Commit

Permalink
Remove chrome://settings-frame on CrOS
Browse files Browse the repository at this point in the history 
This removes all remaining references to chrome://settings-frame
and removes the URL.

It does not remove the options code yet, see the list of issues
blocking crbug.com/728353

This also removes quick_unlock_configure_overlay.js  since it
included a reference to chrome://settings-frame and is only
used by the now fully deprecated options UI.

Bug: 748164
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I1b44f8a9c4b4eeefb972ae4b29eec95a8969220c
Reviewed-on: https://chromium-review.googlesource.com/583935
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489672}
  • Loading branch information
@stevenjb
stevenjb authored and Commit Bot committed Jul 26, 2017
1 parent 247d8bf commit eaa2161
Show file tree
Hide file tree
Showing 14 changed files with 24 additions and 593 deletions.
292 changes: 2 additions & 290 deletions chrome/browser/policy/policy_prefs_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,115 +374,6 @@ class PolicyTestCases {
DISALLOW_COPY_AND_ASSIGN(PolicyTestCases);
};

#if defined(OS_CHROMEOS)

// Returns a pseudo-random integer distributed in [0, range).
int GetRandomNumber(int range) {
return rand() % range;
}

// Splits all known policies into subsets of the given |chunk_size|. The
// policies are shuffled so that there is no correlation between their initial
// alphabetic ordering and the assignment to chunks. This ensures that the
// expected number of policies with long-running test cases is equal for each
// subset. The shuffle algorithm uses a fixed seed, ensuring that no randomness
// is introduced into the testing process.
std::vector<std::vector<std::string> > SplitPoliciesIntoChunks(int chunk_size) {
Schema chrome_schema = Schema::Wrap(GetChromeSchemaData());
if (!chrome_schema.valid())
ADD_FAILURE();

std::vector<std::string> policies;
for (Schema::Iterator it = chrome_schema.GetPropertiesIterator();
!it.IsAtEnd(); it.Advance()) {
policies.push_back(it.key());
}

// Use a fixed random seed to obtain a reproducible shuffle.
srand(1);
std::random_shuffle(policies.begin(), policies.end(), GetRandomNumber);

std::vector<std::vector<std::string> > chunks;
std::vector<std::string>::const_iterator it = policies.begin();
const std::vector<std::string>::const_iterator end = policies.end();
for ( ; end - it >= chunk_size; it += chunk_size)
chunks.push_back(std::vector<std::string>(it, it + chunk_size));
if (it != end)
chunks.push_back(std::vector<std::string>(it, end));
return chunks;
}

void VerifyControlledSettingIndicators(Browser* browser,
const std::string& selector,
const std::string& value,
const std::string& controlled_by,
bool readonly) {
std::stringstream javascript;
javascript << "var nodes = document.querySelectorAll("
<< " 'span.controlled-setting-indicator"
<< selector.c_str() << "');"
<< "var indicators = [];"
<< "for (var i = 0; i < nodes.length; i++) {"
<< " var node = nodes[i];"
<< " var indicator = {};"
<< " indicator.value = node.value || '';"
<< " indicator.controlledBy = node.controlledBy || '';"
<< " indicator.readOnly = node.readOnly || false;"
<< " indicator.visible ="
<< " window.getComputedStyle(node).display != 'none';"
<< " indicators.push(indicator)"
<< "}"
<< "domAutomationController.send(JSON.stringify(indicators));";
content::WebContents* contents =
browser->tab_strip_model()->GetActiveWebContents();
std::string json;
// Retrieve the state of all controlled setting indicators matching the
// |selector| as JSON.
ASSERT_TRUE(content::ExecuteScriptAndExtractString(contents, javascript.str(),
&json));
std::unique_ptr<base::Value> value_ptr = base::JSONReader::Read(json);
const base::ListValue* indicators = NULL;
ASSERT_TRUE(value_ptr.get());
ASSERT_TRUE(value_ptr->GetAsList(&indicators));
// Verify that controlled setting indicators representing |value| are visible
// and have the correct state while those not representing |value| are
// invisible.
if (!controlled_by.empty()) {
EXPECT_GT(indicators->GetSize(), 0u)
<< "Expected to find at least one controlled setting indicator.";
}
bool have_visible_indicators = false;
for (base::ListValue::const_iterator indicator = indicators->begin();
indicator != indicators->end(); ++indicator) {
const base::DictionaryValue* properties = NULL;
ASSERT_TRUE(indicator->GetAsDictionary(&properties));
std::string indicator_value;
std::string indicator_controlled_by;
bool indicator_readonly;
bool indicator_visible;
EXPECT_TRUE(properties->GetString("value", &indicator_value));
EXPECT_TRUE(properties->GetString("controlledBy",
&indicator_controlled_by));
EXPECT_TRUE(properties->GetBoolean("readOnly", &indicator_readonly));
EXPECT_TRUE(properties->GetBoolean("visible", &indicator_visible));
if (!controlled_by.empty() && (indicator_value == value)) {
EXPECT_EQ(controlled_by, indicator_controlled_by);
EXPECT_EQ(readonly, indicator_readonly);
EXPECT_TRUE(indicator_visible);
have_visible_indicators = true;
} else {
EXPECT_FALSE(indicator_visible);
}
}
if (!controlled_by.empty()) {
EXPECT_TRUE(have_visible_indicators)
<< "Expected to find at least one visible controlled setting "
<< "indicator.";
}
}

#endif // defined(OS_CHROMEOS)

} // namespace

typedef InProcessBrowserTest PolicyPrefsTestCoverageTest;
Expand Down Expand Up @@ -602,186 +493,7 @@ IN_PROC_BROWSER_TEST_F(PolicyPrefsTest, PolicyToPrefsMapping) {
}
}

class PolicyPrefIndicatorTest
: public PolicyPrefsTest,
public testing::WithParamInterface<std::vector<std::string> > {
};

#if defined(OS_CHROMEOS)

// Verifies that controlled setting indicators correctly show whether a pref's
// value is recommended or enforced by a corresponding policy.
IN_PROC_BROWSER_TEST_P(PolicyPrefIndicatorTest, CheckPolicyIndicators) {
const PolicyTestCases test_cases;
PrefService* local_state = g_browser_process->local_state();
PrefService* user_prefs = browser()->profile()->GetPrefs();

ui_test_utils::NavigateToURL(browser(),
GURL(chrome::kChromeUISettingsFrameURL));

for (std::vector<std::string>::const_iterator policy = GetParam().begin();
policy != GetParam().end();
++policy) {
const std::vector<PolicyTestCase*>* policy_test_cases =
test_cases.Get(*policy);
ASSERT_TRUE(policy_test_cases) << "PolicyTestCase not found for "
<< *policy;
for (std::vector<PolicyTestCase*>::const_iterator test_case =
policy_test_cases->begin();
test_case != policy_test_cases->end();
++test_case) {
PolicyTestCase* policy_test_case = *test_case;
if (!policy_test_case->IsSupported())
continue;
const auto& pref_mappings = policy_test_case->pref_mappings();
if (policy_test_case->indicator_selector().empty()) {
bool has_pref_indicator_tests = false;
for (const auto& pref_mapping : pref_mappings) {
PrefService* prefs =
pref_mapping->is_local_state() ? local_state : user_prefs;
if (prefs->FindPreference(pref_mapping->pref()))
prefs->ClearPref(pref_mapping->pref());
if (!pref_mapping->indicator_test_cases().empty()) {
has_pref_indicator_tests = true;
break;
}
}
if (!has_pref_indicator_tests)
continue;
}

LOG(INFO) << "Testing policy: " << *policy;

if (!policy_test_case->indicator_selector().empty()) {
// Check that no controlled setting indicator is visible when no value
// is set by policy.
ClearProviderPolicy();
VerifyControlledSettingIndicators(
browser(),
policy_test_case->indicator_selector(),
std::string(),
std::string(),
false);
// Check that the appropriate controlled setting indicator is shown when
// a value is enforced by policy.
SetProviderPolicy(policy_test_case->test_policy(),
POLICY_LEVEL_MANDATORY);
VerifyControlledSettingIndicators(
browser(),
policy_test_case->indicator_selector(),
std::string(),
"policy",
false);
// Check that no controlled setting indicator is visible when previously
// enforced value is removed.
ClearProviderPolicy();
VerifyControlledSettingIndicators(
browser(),
policy_test_case->indicator_selector(),
std::string(),
std::string(),
false);
}

for (const auto& pref_mapping : pref_mappings) {
const auto& indicator_test_cases = pref_mapping->indicator_test_cases();
if (indicator_test_cases.empty())
continue;

if (!pref_mapping->indicator_test_setup_js().empty()) {
ASSERT_TRUE(content::ExecuteScript(
browser()->tab_strip_model()->GetActiveWebContents(),
pref_mapping->indicator_test_setup_js()));
}

// A non-empty indicator_test_url is expected to be used in very
// few cases, so it's currently implemented by navigating to the URL
// right before the test and navigating back afterwards.
// If you introduce many test cases with the same non-empty
// indicator_test_url, this would be inefficient. We could consider
// navigting to a specific indicator_test_url once for many test cases
// instead.
if (!pref_mapping->indicator_test_url().empty()) {
ui_test_utils::NavigateToURL(
browser(), GURL(pref_mapping->indicator_test_url()));
}

std::string indicator_selector = pref_mapping->indicator_selector();
if (indicator_selector.empty())
indicator_selector = "[pref=\"" + pref_mapping->pref() + "\"]";
for (auto indicator_test_case = indicator_test_cases.begin();
indicator_test_case != indicator_test_cases.end();
++indicator_test_case) {
// Check that no controlled setting indicator is visible when no value
// is set by policy.
ClearProviderPolicy();
VerifyControlledSettingIndicators(browser(),
indicator_selector,
std::string(),
std::string(),
false);

if (pref_mapping->check_for_mandatory()) {
// Check that the appropriate controlled setting indicator is shown
// when a value is enforced by policy.
SetProviderPolicy((*indicator_test_case)->policy(),
POLICY_LEVEL_MANDATORY);

VerifyControlledSettingIndicators(
browser(),
indicator_selector,
(*indicator_test_case)->value(),
"policy",
(*indicator_test_case)->readonly());
}

if (!policy_test_case->can_be_recommended() ||
!pref_mapping->check_for_recommended()) {
continue;
}

PrefService* prefs =
pref_mapping->is_local_state() ? local_state : user_prefs;
// The preference must have been registered.
const PrefService::Preference* pref =
prefs->FindPreference(pref_mapping->pref().c_str());
ASSERT_TRUE(pref);

// Check that the appropriate controlled setting indicator is shown
// when a value is recommended by policy and the user has not
// overridden the recommendation.
SetProviderPolicy((*indicator_test_case)->policy(),
POLICY_LEVEL_RECOMMENDED);
VerifyControlledSettingIndicators(browser(),
indicator_selector,
(*indicator_test_case)->value(),
"recommended",
(*indicator_test_case)->readonly());
// Check that the appropriate controlled setting indicator is shown
// when a value is recommended by policy and the user has overridden
// the recommendation.
prefs->Set(pref_mapping->pref().c_str(), *pref->GetValue());
VerifyControlledSettingIndicators(browser(),
indicator_selector,
(*indicator_test_case)->value(),
"hasRecommendation",
(*indicator_test_case)->readonly());
prefs->ClearPref(pref_mapping->pref().c_str());
}

if (!pref_mapping->indicator_test_url().empty()) {
ui_test_utils::NavigateToURL(browser(),
GURL(chrome::kChromeUISettingsFrameURL));
}
}
}
}
}

INSTANTIATE_TEST_CASE_P(PolicyPrefIndicatorTestInstance,
PolicyPrefIndicatorTest,
testing::ValuesIn(SplitPoliciesIntoChunks(10)));

#endif // defined(OS_CHROMEOS)
// For WebUI integration tests, see cr_policy_indicator_tests.js and
// cr_policy_pref_indicator_tests.js.

} // namespace policy
1 change: 0 additions & 1 deletion chrome/browser/ui/browser_navigator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,6 @@ bool IsURLAllowedInIncognito(const GURL& url,
if (url.scheme() == content::kChromeUIScheme &&
(url.host_piece() == chrome::kChromeUISettingsHost ||
url.host_piece() == chrome::kChromeUIMdSettingsHost ||
url.host_piece() == chrome::kChromeUISettingsFrameHost ||
url.host_piece() == chrome::kChromeUIHelpHost ||
url.host_piece() == chrome::kChromeUIHistoryHost ||
url.host_piece() == chrome::kChromeUIExtensionsHost ||
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/ui/chrome_pages.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,7 @@ GURL GetSettingsUrl(const std::string& sub_page) {

bool IsSettingsSubPage(const GURL& url, const std::string& sub_page) {
return (url.SchemeIs(content::kChromeUIScheme) &&
(url.host_piece() == chrome::kChromeUISettingsHost ||
url.host_piece() == chrome::kChromeUISettingsFrameHost) &&
(url.host_piece() == chrome::kChromeUISettingsHost) &&
url.path_piece() == "/" + sub_page);
}

Expand Down
4 changes: 0 additions & 4 deletions chrome/browser/ui/content_settings/content_setting_bubble_model_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,6 @@ IN_PROC_BROWSER_TEST_F(ContentSettingBubbleModelMediaStreamTest,
EXPECT_EQ(GURL("chrome://settings/content#media-stream-mic"),
GetActiveTab()->GetLastCommittedURL());

// In ChromeOS, we do not sanitize chrome://settings-frame to
// chrome://settings for same-document navigations. See crbug.com/416157. For
// this reason, order the tests so no same-document navigation occurs.

// The camera bubble links to camera exceptions.
ManageMediaStreamSettings(TabSpecificContentSettings::CAMERA_ACCESSED);
EXPECT_EQ(GURL("chrome://settings/contentExceptions#media-stream-camera"),
Expand Down
4 changes: 0 additions & 4 deletions chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@
#include "chrome/browser/ui/webui/chromeos/sim_unlock_ui.h"
#include "chrome/browser/ui/webui/chromeos/slow_trace_ui.h"
#include "chrome/browser/ui/webui/chromeos/slow_ui.h"
#include "chrome/browser/ui/webui/options/options_ui.h"
#include "chrome/browser/ui/webui/voice_search_ui.h"
#include "components/proximity_auth/webui/proximity_auth_ui.h"
#include "components/proximity_auth/webui/url_constants.h"
Expand Down Expand Up @@ -480,8 +479,6 @@ WebUIFactoryFunction GetWebUIFactoryFunction(WebUI* web_ui,
return &NewWebUI<chromeos::ProxySettingsUI>;
if (url.host_piece() == chrome::kChromeUISetTimeHost)
return &NewWebUI<chromeos::SetTimeUI>;
if (url.host_piece() == chrome::kChromeUISettingsFrameHost)
return &NewWebUI<options::OptionsUI>;
if (url.host_piece() == chrome::kChromeUISimUnlockHost)
return &NewWebUI<chromeos::SimUnlockUI>;
if (url.host_piece() == chrome::kChromeUISlowHost)
Expand Down Expand Up @@ -814,7 +811,6 @@ base::RefCountedMemory* ChromeWebUIControllerFactory::GetFaviconResourceBytes(

// Android doesn't use the Options/Settings pages.
if (page_url.host_piece() == chrome::kChromeUISettingsHost ||
page_url.host_piece() == chrome::kChromeUISettingsFrameHost ||
page_url.host_piece() == chrome::kChromeUIMdSettingsHost)
return settings_utils::GetFaviconResourceBytes(scale_factor);

Expand Down
9 changes: 6 additions & 3 deletions chrome/browser/ui/webui/options/options_ui.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,11 @@ OptionsUIHTMLSource::OptionsUIHTMLSource(
}

std::string OptionsUIHTMLSource::GetSource() const {
return chrome::kChromeUISettingsFrameHost;
// TODO(stevenjb): Remove this file. Because everything in this directory
// depends on this, we will remove the entire directory at once after
// the old CrOS oobe/login UI dependencies are removed. crbug.com/748164.
NOTREACHED();
return "settings-frame";
}

void OptionsUIHTMLSource::StartDataRequest(
Expand Down Expand Up @@ -591,8 +595,7 @@ void OptionsUI::ReadyToCommitNavigation(
load_start_time_ = base::Time::Now();
if (navigation_handle->GetRenderFrameHost()->GetRenderViewHost() ==
web_ui()->GetWebContents()->GetRenderViewHost() &&
navigation_handle->GetURL().host_piece() ==
chrome::kChromeUISettingsFrameHost) {
navigation_handle->GetURL().host_piece() == "settings-frame") {
for (size_t i = 0; i < handlers_.size(); ++i)
handlers_[i]->PageLoadStarted();
}
Expand Down
Loading

0 comments on commit eaa2161

Please sign in to comment.