Skip to content

Commit

Permalink
Remove migration of obsolete value for "session.restore_on_startup".
Browse files Browse the repository at this point in the history 
Remove support of migration of kPrefValueHomePage (0) for the preference
"session.restore_on_startup" that was obsolete when M-19 was released.

Remove all usage of the constant kPrefValueHomePage and of the preference
"session.restore_on_startup_migrated" tracking whether the migration had
happened.

BUG=525079

Review URL: https://codereview.chromium.org/1312693005

Cr-Commit-Position: refs/heads/master@{#364142}
  • Loading branch information
@sdefresne
sdefresne authored and Commit bot committed Dec 9, 2015
1 parent 883fe5b commit 245bbd4
Show file tree
Hide file tree
Showing 12 changed files with 25 additions and 392 deletions.
54 changes: 1 addition & 53 deletions chrome/browser/policy/policy_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3269,56 +3269,6 @@ class RestoreOnStartupPolicyTest
RedirectHostsToTestData, kRestoredURLs, arraysize(kRestoredURLs)));
}

void HomepageIsNotNTP() {
// Verifies that policy can set the startup pages to the homepage, when
// the homepage is not the NTP.
PolicyMap policies;
policies.Set(
key::kRestoreOnStartup,
POLICY_LEVEL_MANDATORY,
POLICY_SCOPE_USER,
POLICY_SOURCE_CLOUD,
new base::FundamentalValue(SessionStartupPref::kPrefValueHomePage),
NULL);
policies.Set(key::kHomepageIsNewTabPage,
POLICY_LEVEL_MANDATORY,
POLICY_SCOPE_USER,
POLICY_SOURCE_CLOUD,
new base::FundamentalValue(false),
NULL);
policies.Set(key::kHomepageLocation,
POLICY_LEVEL_MANDATORY,
POLICY_SCOPE_USER,
POLICY_SOURCE_CLOUD,
new base::StringValue(kRestoredURLs[1]),
NULL);
provider_.UpdateChromePolicy(policies);

expected_urls_.push_back(GURL(kRestoredURLs[1]));
}

void HomepageIsNTP() {
// Verifies that policy can set the startup pages to the homepage, when
// the homepage is the NTP.
PolicyMap policies;
policies.Set(
key::kRestoreOnStartup,
POLICY_LEVEL_MANDATORY,
POLICY_SCOPE_USER,
POLICY_SOURCE_CLOUD,
new base::FundamentalValue(SessionStartupPref::kPrefValueHomePage),
NULL);
policies.Set(key::kHomepageIsNewTabPage,
POLICY_LEVEL_MANDATORY,
POLICY_SCOPE_USER,
POLICY_SOURCE_CLOUD,
new base::FundamentalValue(true),
NULL);
provider_.UpdateChromePolicy(policies);

expected_urls_.push_back(GURL(chrome::kChromeUINewTabURL));
}

void ListOfURLs() {
// Verifies that policy can set the startup pages to a list of URLs.
base::ListValue urls;
Expand Down Expand Up @@ -3405,9 +3355,7 @@ IN_PROC_BROWSER_TEST_P(RestoreOnStartupPolicyTest, RunTest) {
INSTANTIATE_TEST_CASE_P(
RestoreOnStartupPolicyTestInstance,
RestoreOnStartupPolicyTest,
testing::Values(&RestoreOnStartupPolicyTest::HomepageIsNotNTP,
&RestoreOnStartupPolicyTest::HomepageIsNTP,
&RestoreOnStartupPolicyTest::ListOfURLs,
testing::Values(&RestoreOnStartupPolicyTest::ListOfURLs,
&RestoreOnStartupPolicyTest::NTP,
&RestoreOnStartupPolicyTest::Last));

Expand Down
7 changes: 7 additions & 0 deletions chrome/browser/prefs/browser_prefs.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ const char kURLsToRestoreOnStartupOld[] = "session.urls_to_restore_on_startup";
const char kRestoreStartupURLsMigrationTime[] =
"session.startup_urls_migration_time";

// Deprecated 12/2015.
const char kRestoreOnStartupMigrated[] = "session.restore_on_startup_migrated";

} // namespace

namespace chrome {
Expand Down Expand Up @@ -542,6 +545,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {

registry->RegisterListPref(kURLsToRestoreOnStartupOld);
registry->RegisterInt64Pref(kRestoreStartupURLsMigrationTime, 0);
registry->RegisterBooleanPref(kRestoreOnStartupMigrated, false);
}

void RegisterUserProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
Expand Down Expand Up @@ -593,6 +597,9 @@ void MigrateObsoleteProfilePrefs(Profile* profile) {
// Added 12/1015.
profile_prefs->ClearPref(kURLsToRestoreOnStartupOld);
profile_prefs->ClearPref(kRestoreStartupURLsMigrationTime);

// Added 12/2015.
profile_prefs->ClearPref(kRestoreOnStartupMigrated);
}

} // namespace chrome
62 changes: 0 additions & 62 deletions chrome/browser/prefs/session_startup_pref.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,6 @@ int TypeToPrefValue(SessionStartupPref::Type type) {
}
}

void SetNewURLList(PrefService* prefs) {
if (prefs->IsUserModifiablePreference(prefs::kURLsToRestoreOnStartup)) {
base::ListValue new_url_pref_list;
base::StringValue* home_page =
new base::StringValue(prefs->GetString(prefs::kHomePage));
new_url_pref_list.Append(home_page);
prefs->Set(prefs::kURLsToRestoreOnStartup, new_url_pref_list);
}
}

void URLListToPref(const base::ListValue* url_list, SessionStartupPref* pref) {
pref->urls.clear();
for (size_t i = 0; i < url_list->GetSize(); ++i) {
Expand All @@ -57,7 +47,6 @@ void SessionStartupPref::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
registry->RegisterListPref(prefs::kURLsToRestoreOnStartup,
user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
registry->RegisterBooleanPref(prefs::kRestoreOnStartupMigrated, false);
}

// static
Expand Down Expand Up @@ -110,8 +99,6 @@ SessionStartupPref SessionStartupPref::GetStartupPref(Profile* profile) {
SessionStartupPref SessionStartupPref::GetStartupPref(PrefService* prefs) {
DCHECK(prefs);

MigrateIfNecessary(prefs);

SessionStartupPref pref(
PrefValueToType(prefs->GetInteger(prefs::kRestoreOnStartup)));

Expand All @@ -124,54 +111,6 @@ SessionStartupPref SessionStartupPref::GetStartupPref(PrefService* prefs) {
return pref;
}

// static
void SessionStartupPref::MigrateIfNecessary(PrefService* prefs) {
DCHECK(prefs);

if (!prefs->GetBoolean(prefs::kRestoreOnStartupMigrated)) {
// Read existing values.
const base::Value* homepage_is_new_tab_page_value =
prefs->GetUserPrefValue(prefs::kHomePageIsNewTabPage);
bool homepage_is_new_tab_page = true;
if (homepage_is_new_tab_page_value) {
if (!homepage_is_new_tab_page_value->GetAsBoolean(
&homepage_is_new_tab_page))
NOTREACHED();
}

const base::Value* restore_on_startup_value =
prefs->GetUserPrefValue(prefs::kRestoreOnStartup);
int restore_on_startup = -1;
if (restore_on_startup_value) {
if (!restore_on_startup_value->GetAsInteger(&restore_on_startup))
NOTREACHED();
}

// If restore_on_startup has the deprecated value kPrefValueHomePage,
// migrate it to open the homepage on startup. If 'homepage is NTP' is set,
// that means just opening the NTP. If not, it means opening a one-item URL
// list containing the homepage.
if (restore_on_startup == kPrefValueHomePage) {
if (homepage_is_new_tab_page) {
prefs->SetInteger(prefs::kRestoreOnStartup, kPrefValueNewTab);
} else {
prefs->SetInteger(prefs::kRestoreOnStartup, kPrefValueURLs);
SetNewURLList(prefs);
}
} else if (!restore_on_startup_value && !homepage_is_new_tab_page &&
GetDefaultStartupType() == DEFAULT) {
// kRestoreOnStartup was never set by the user, but the homepage was set.
// Migrate to the list of URLs. (If restore_on_startup was never set,
// and homepage_is_new_tab_page is true, no action is needed. The new
// default value is "open the new tab page" which is what we want.)
prefs->SetInteger(prefs::kRestoreOnStartup, kPrefValueURLs);
SetNewURLList(prefs);
}

prefs->SetBoolean(prefs::kRestoreOnStartupMigrated, true);
}
}

// static
bool SessionStartupPref::TypeIsManaged(PrefService* prefs) {
DCHECK(prefs);
Expand Down Expand Up @@ -204,7 +143,6 @@ SessionStartupPref::Type SessionStartupPref::PrefValueToType(int pref_value) {
switch (pref_value) {
case kPrefValueLast: return SessionStartupPref::LAST;
case kPrefValueURLs: return SessionStartupPref::URLS;
case kPrefValueHomePage: return SessionStartupPref::HOMEPAGE;
default: return SessionStartupPref::DEFAULT;
}
}
Expand Down
12 changes: 0 additions & 12 deletions chrome/browser/prefs/session_startup_pref.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,18 @@ struct SessionStartupPref {
// Indicates the user wants to open the New Tab page.
DEFAULT,

// Deprecated. See comment in session_startup_pref.cc
HOMEPAGE,

// Indicates the user wants to restore the last session.
LAST,

// Indicates the user wants to restore a specific set of URLs. The URLs
// are contained in urls.
URLS,

// Number of values in this enum.
TYPE_COUNT
};

// For historical reasons the enum and value registered in the prefs don't
// line up. These are the values registered in prefs.
// The values are also recorded in Settings.StartupPageLoadSettings histogram,
// so make sure to update histograms.xml if you change these.
static const int kPrefValueHomePage = 0; // Deprecated
static const int kPrefValueLast = 1;
static const int kPrefValueURLs = 4;
static const int kPrefValueNewTab = 5;
Expand All @@ -59,11 +52,6 @@ struct SessionStartupPref {
static SessionStartupPref GetStartupPref(Profile* profile);
static SessionStartupPref GetStartupPref(PrefService* prefs);

// If the user had the "restore on startup" property set to the deprecated
// "Open the home page" value, this migrates them to a value that will have
// the same effect.
static void MigrateIfNecessary(PrefService* prefs);

// Whether the startup type and URLs are managed via policy.
static bool TypeIsManaged(PrefService* prefs);
static bool URLsAreManaged(PrefService* prefs);
Expand Down
94 changes: 0 additions & 94 deletions chrome/browser/prefs/session_startup_pref_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,6 @@ class SessionStartupPrefTest : public testing::Test {
registry()->RegisterBooleanPref(prefs::kHomePageIsNewTabPage, true);
}

bool IsUseLastOpenDefault() {
// On ChromeOS, the default SessionStartupPref is LAST.
#if defined(OS_CHROMEOS)
return true;
#else
return false;
#endif
}

user_prefs::PrefRegistrySyncable* registry() {
return pref_service_->registry();
}
Expand Down Expand Up @@ -71,88 +62,3 @@ TEST_F(SessionStartupPrefTest, URLListManagedOverridesUser) {
result = SessionStartupPref::GetStartupPref(pref_service_.get());
EXPECT_EQ(3u, result.urls.size());
}

// Checks to make sure that if the user had previously not selected anything
// (so that, in effect, the default value "Open the homepage" was selected),
// their preferences are migrated on upgrade to m19.
TEST_F(SessionStartupPrefTest, DefaultMigration) {
registry()->RegisterStringPref(prefs::kHomePage, "http://google.com/");
pref_service_->SetString(prefs::kHomePage, "http://chromium.org/");
pref_service_->SetBoolean(prefs::kHomePageIsNewTabPage, false);

EXPECT_FALSE(pref_service_->HasPrefPath(prefs::kRestoreOnStartup));

SessionStartupPref pref = SessionStartupPref::GetStartupPref(
pref_service_.get());

if (IsUseLastOpenDefault()) {
EXPECT_EQ(SessionStartupPref::LAST, pref.type);
EXPECT_EQ(0U, pref.urls.size());
} else {
EXPECT_EQ(SessionStartupPref::URLS, pref.type);
EXPECT_EQ(1U, pref.urls.size());
EXPECT_EQ(GURL("http://chromium.org/"), pref.urls[0]);
}
}

// Checks to make sure that if the user had previously not selected anything
// (so that, in effect, the default value "Open the homepage" was selected),
// and the NTP is being used for the homepage, their preferences are migrated
// to "Open the New Tab Page" on upgrade to M19.
TEST_F(SessionStartupPrefTest, DefaultMigrationHomepageIsNTP) {
registry()->RegisterStringPref(prefs::kHomePage, "http://google.com/");
pref_service_->SetString(prefs::kHomePage, "http://chromium.org/");
pref_service_->SetBoolean(prefs::kHomePageIsNewTabPage, true);

EXPECT_FALSE(pref_service_->HasPrefPath(prefs::kRestoreOnStartup));

SessionStartupPref pref = SessionStartupPref::GetStartupPref(
pref_service_.get());

if (IsUseLastOpenDefault())
EXPECT_EQ(SessionStartupPref::LAST, pref.type);
else
EXPECT_EQ(SessionStartupPref::DEFAULT, pref.type);

// The "URLs to restore on startup" shouldn't get migrated.
EXPECT_EQ(0U, pref.urls.size());
}

// Checks to make sure that if the user had previously selected "Open the
// "homepage", their preferences are migrated on upgrade to M19.
TEST_F(SessionStartupPrefTest, HomePageMigration) {
registry()->RegisterStringPref(prefs::kHomePage, "http://google.com/");

// By design, it's impossible to set the 'restore on startup' pref to 0
// ("open the homepage") using SessionStartupPref::SetStartupPref(), so set it
// using the pref service directly.
pref_service_->SetInteger(prefs::kRestoreOnStartup, /*kPrefValueHomePage*/ 0);
pref_service_->SetString(prefs::kHomePage, "http://chromium.org/");
pref_service_->SetBoolean(prefs::kHomePageIsNewTabPage, false);

SessionStartupPref pref = SessionStartupPref::GetStartupPref(
pref_service_.get());

EXPECT_EQ(SessionStartupPref::URLS, pref.type);
EXPECT_EQ(1U, pref.urls.size());
EXPECT_EQ(GURL("http://chromium.org/"), pref.urls[0]);
}

// Checks to make sure that if the user had previously selected "Open the
// "homepage", and the NTP is being used for the homepage, their preferences
// are migrated on upgrade to M19.
TEST_F(SessionStartupPrefTest, HomePageMigrationHomepageIsNTP) {
registry()->RegisterStringPref(prefs::kHomePage, "http://google.com/");

// By design, it's impossible to set the 'restore on startup' pref to 0
// ("open the homepage") using SessionStartupPref::SetStartupPref(), so set it
// using the pref service directly.
pref_service_->SetInteger(prefs::kRestoreOnStartup, /*kPrefValueHomePage*/ 0);
pref_service_->SetString(prefs::kHomePage, "http://chromium.org/");
pref_service_->SetBoolean(prefs::kHomePageIsNewTabPage, true);

SessionStartupPref pref = SessionStartupPref::GetStartupPref(
pref_service_.get());

EXPECT_EQ(SessionStartupPref::DEFAULT, pref.type);
}
1 change: 0 additions & 1 deletion chrome/browser/profile_resetter/profile_resetter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ void ProfileResetter::ResetStartupPages() {
else
prefs->ClearPref(prefs::kRestoreOnStartup);

prefs->SetBoolean(prefs::kRestoreOnStartupMigrated, true);
MarkAsDone(STARTUP_PAGES);
}

Expand Down
42 changes: 2 additions & 40 deletions chrome/browser/sessions/restore_on_startup_policy_handler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,45 +32,7 @@ void RestoreOnStartupPolicyHandler::ApplyPolicySettings(
int restore_on_startup;
if (!restore_on_startup_value->GetAsInteger(&restore_on_startup))
return;

if (restore_on_startup == SessionStartupPref::kPrefValueHomePage)
ApplyPolicySettingsFromHomePage(policies, prefs);
else
prefs->SetInteger(prefs::kRestoreOnStartup, restore_on_startup);
}
}

void RestoreOnStartupPolicyHandler::ApplyPolicySettingsFromHomePage(
const PolicyMap& policies,
PrefValueMap* prefs) {
const base::Value* homepage_is_new_tab_page_value =
policies.GetValue(key::kHomepageIsNewTabPage);
if (!homepage_is_new_tab_page_value) {
// The policy is enforcing 'open the homepage on startup' but not
// enforcing what the homepage should be. Don't set any prefs.
return;
}

bool homepage_is_new_tab_page;
if (!homepage_is_new_tab_page_value->GetAsBoolean(&homepage_is_new_tab_page))
return;

if (homepage_is_new_tab_page) {
prefs->SetInteger(prefs::kRestoreOnStartup,
SessionStartupPref::kPrefValueNewTab);
} else {
const base::Value* homepage_value =
policies.GetValue(key::kHomepageLocation);
if (!homepage_value || !homepage_value->IsType(base::Value::TYPE_STRING)) {
// The policy is enforcing 'open the homepage on startup' but not
// enforcing what the homepage should be. Don't set any prefs.
return;
}
scoped_ptr<base::ListValue> url_list(new base::ListValue());
url_list->Append(homepage_value->CreateDeepCopy());
prefs->SetInteger(prefs::kRestoreOnStartup,
SessionStartupPref::kPrefValueURLs);
prefs->SetValue(prefs::kURLsToRestoreOnStartup, url_list.Pass());
prefs->SetInteger(prefs::kRestoreOnStartup, restore_on_startup);
}
}

Expand All @@ -86,7 +48,7 @@ bool RestoreOnStartupPolicyHandler::CheckPolicySettings(
int restore_value;
CHECK(restore_policy->GetAsInteger(&restore_value)); // Passed type check.
switch (restore_value) {
case SessionStartupPref::kPrefValueHomePage:
case 0: // Deprecated kPrefValueHomePage.
errors->AddError(policy_name(), IDS_POLICY_VALUE_DEPRECATED);
break;
case SessionStartupPref::kPrefValueLast: {
Expand Down
Loading

0 comments on commit 245bbd4

Please sign in to comment.