Skip to content

Commit

Permalink
sync: add WIFI_CREDENTIALS protobuf, ModelType, and preference
Browse files Browse the repository at this point in the history
- add a protobuf for WiFi credentials
- add a ModelType for syncing WiFi credentials
- add UI for controlling whether or not WiFi credentials are synced
- add new data type to testserver

While there:
- update comment in sync_setup_overlay.js
- fix ordering of arguments to EXPECT_EQ in CheckBool
  (sync_setup_handler_unittest)
- move GetSelectableTypeNameMap from sync_setup_handler.cc to model_type.cc

BUG=chromium:422045
TEST=ProfileSync, SyncSetup, SyncModel, ModelType, NigoriUtil,
     ProtoEnumConversions, ProtoValueConversions,
     SyncEncryptionHandlerImpl

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

Cr-Commit-Position: refs/heads/master@{#302159}
  • Loading branch information
quiche authored and Commit bot committed Oct 30, 2014
1 parent 00dc0d4 commit dfad17b
Show file tree
Hide file tree
Showing 29 changed files with 271 additions and 65 deletions.
3 changes: 3 additions & 0 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -12376,6 +12376,9 @@ Some features may be unavailable. Please check that the profile exists and you
<message name="IDS_SYNC_DATATYPE_TABS" desc="Open Tabs, one of the data types that we allow syncing.">
Open Tabs
</message>
<message name="IDS_SYNC_DATATYPE_WIFI_CREDENTIALS" desc="WiFi network names and passwords, one of the data types that we allow syncing.">
WiFi Credentials
</message>

<!-- Encryption tab of the configure sync dialog -->
<message name="IDS_SYNC_ENCRYPTION_INSTRUCTIONS" desc="Instructions for the encryption settings tab.">
Expand Down
7 changes: 7 additions & 0 deletions chrome/browser/resources/sync_setup_overlay.html
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,13 @@ <h1 i18n-content="syncSetupConfigureTitle"></h1>
</span>
</label>
</div>
<div id="wifi-credentials-item" class="sync-type-checkbox checkbox">
<label>
<input id="wifi-credentials-checkbox" type="checkbox">
<span i18n-content="wifiCredentials" il8n-values="title:wifiCredentials">
</span>
</label>
</div>
</div>
</div>
<div id="customize-sync-encryption-new">
Expand Down
15 changes: 14 additions & 1 deletion chrome/browser/resources/sync_setup_overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ cr.define('options', function() {
$('use-default-link').onclick = null;

// These values need to be kept in sync with where they are read in
// SyncSetupFlow::GetDataTypeChoiceData().
// sync_setup_handler.cc:GetConfiguration().
var syncAll = $('sync-select-datatypes').selectedIndex ==
options.DataTypeSelection.SYNC_EVERYTHING;
var syncNothing = $('sync-select-datatypes').selectedIndex ==
Expand All @@ -323,6 +323,8 @@ cr.define('options', function() {
'typedUrlsSynced': syncAll || $('typed-urls-checkbox').checked,
'appsSynced': syncAll || $('apps-checkbox').checked,
'tabsSynced': syncAll || $('tabs-checkbox').checked,
'wifiCredentialsSynced': syncAll ||
$('wifi-credentials-checkbox').checked,
'encryptAllData': encryptAllData,
'usePassphrase': usePassphrase,
'isGooglePassphrase': googlePassphrase,
Expand Down Expand Up @@ -439,6 +441,17 @@ cr.define('options', function() {
} else {
$('tabs-item').hidden = true;
}
if (args.wifiCredentialsRegistered) {
$('wifi-credentials-checkbox').checked = args.wifiCredentialsSynced;
dataTypeBoxesChecked_['wifi-credentials-checkbox'] =
args.wifiCredentialsSynced;
dataTypeBoxesDisabled_['wifi-credentials-checkbox'] =
args.wifiCredentialsEnforced;
$('wifi-credentials-checkbox').onclick = this.handleDataTypeClick_;
$('wifi-credentials-item').hidden = false;
} else {
$('wifi-credentials-item').hidden = true;
}

this.setDataTypeCheckboxes_(datatypeSelect.selectedIndex);
},
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/sync/profile_sync_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1738,10 +1738,11 @@ void ProfileSyncService::UpdateSelectedTypesHistogram(
sync_driver::user_selectable_type::TYPED_URLS,
sync_driver::user_selectable_type::EXTENSIONS,
sync_driver::user_selectable_type::APPS,
sync_driver::user_selectable_type::PROXY_TABS
sync_driver::user_selectable_type::WIFI_CREDENTIAL,
sync_driver::user_selectable_type::PROXY_TABS,
};

COMPILE_ASSERT(32 == syncer::MODEL_TYPE_COUNT, UpdateCustomConfigHistogram);
COMPILE_ASSERT(33 == syncer::MODEL_TYPE_COUNT, UpdateCustomConfigHistogram);

if (!sync_everything) {
const syncer::ModelTypeSet current_types = GetPreferredDataTypes();
Expand Down
40 changes: 5 additions & 35 deletions chrome/browser/ui/webui/sync_setup_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,37 +91,6 @@ SyncConfigInfo::SyncConfigInfo()

SyncConfigInfo::~SyncConfigInfo() {}

// Note: The order of these types must match the ordering of
// the respective types in ModelType
const char* kDataTypeNames[] = {
"bookmarks",
"preferences",
"passwords",
"autofill",
"themes",
"typedUrls",
"extensions",
"apps",
"tabs"
};

COMPILE_ASSERT(32 == syncer::MODEL_TYPE_COUNT,
update_kDataTypeNames_to_match_UserSelectableTypes);

typedef std::map<syncer::ModelType, const char*> ModelTypeNameMap;

ModelTypeNameMap GetSelectableTypeNameMap() {
ModelTypeNameMap type_names;
syncer::ModelTypeSet type_set = syncer::UserSelectableTypes();
syncer::ModelTypeSet::Iterator it = type_set.First();
DCHECK_EQ(arraysize(kDataTypeNames), type_set.Size());
for (size_t i = 0; i < arraysize(kDataTypeNames) && it.Good();
++i, it.Inc()) {
type_names[it.Get()] = kDataTypeNames[i];
}
return type_names;
}

bool GetConfiguration(const std::string& json, SyncConfigInfo* config) {
scoped_ptr<base::Value> parsed_value(base::JSONReader::Read(json));
base::DictionaryValue* result;
Expand All @@ -143,9 +112,9 @@ bool GetConfiguration(const std::string& json, SyncConfigInfo* config) {
DCHECK(!(config->sync_everything && config->sync_nothing))
<< "syncAllDataTypes and syncNothing cannot both be true";

ModelTypeNameMap type_names = GetSelectableTypeNameMap();
syncer::ModelTypeNameMap type_names = syncer::GetUserSelectableTypeNameMap();

for (ModelTypeNameMap::const_iterator it = type_names.begin();
for (syncer::ModelTypeNameMap::const_iterator it = type_names.begin();
it != type_names.end(); ++it) {
std::string key_name = it->second + std::string("Synced");
bool sync_value;
Expand Down Expand Up @@ -271,6 +240,7 @@ void SyncSetupHandler::GetStaticLocalizedValues(
{ "extensions", IDS_SYNC_DATATYPE_EXTENSIONS },
{ "typedURLs", IDS_SYNC_DATATYPE_TYPED_URLS },
{ "apps", IDS_SYNC_DATATYPE_APPS },
{ "wifiCredentials", IDS_SYNC_DATATYPE_WIFI_CREDENTIALS },
{ "openTabs", IDS_SYNC_DATATYPE_TABS },
{ "serviceUnavailableError", IDS_SYNC_SETUP_ABORTED_BY_PENDING_CLEAR },
{ "confirmLabel", IDS_SYNC_CONFIRM_PASSPHRASE_LABEL },
Expand Down Expand Up @@ -357,8 +327,8 @@ void SyncSetupHandler::DisplayConfigureSync(bool show_advanced,
service->GetRegisteredDataTypes();
const syncer::ModelTypeSet preferred_types = service->GetPreferredDataTypes();
const syncer::ModelTypeSet enforced_types = service->GetForcedDataTypes();
ModelTypeNameMap type_names = GetSelectableTypeNameMap();
for (ModelTypeNameMap::const_iterator it = type_names.begin();
syncer::ModelTypeNameMap type_names = syncer::GetUserSelectableTypeNameMap();
for (syncer::ModelTypeNameMap::const_iterator it = type_names.begin();
it != type_names.end(); ++it) {
syncer::ModelType sync_type = it->first;
const std::string key_name = it->second;
Expand Down
7 changes: 6 additions & 1 deletion chrome/browser/ui/webui/sync_setup_handler_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ std::string GetConfiguration(const base::DictionaryValue* extra_values,
result.SetBoolean("tabsSynced", types.Has(syncer::PROXY_TABS));
result.SetBoolean("themesSynced", types.Has(syncer::THEMES));
result.SetBoolean("typedUrlsSynced", types.Has(syncer::TYPED_URLS));
result.SetBoolean("wifiCredentialsSynced",
types.Has(syncer::WIFI_CREDENTIALS));
std::string args;
base::JSONWriter::Write(&result, &args);
return args;
Expand All @@ -110,7 +112,7 @@ void CheckBool(const base::DictionaryValue* dictionary,
bool actual_value;
EXPECT_TRUE(dictionary->GetBoolean(key, &actual_value)) <<
"No value found for " << key;
EXPECT_EQ(actual_value, expected_value) <<
EXPECT_EQ(expected_value, actual_value) <<
"Mismatch found for " << key;
}
}
Expand Down Expand Up @@ -138,6 +140,8 @@ void CheckConfigDataTypeArguments(base::DictionaryValue* dictionary,
CheckBool(dictionary, "tabsSynced", types.Has(syncer::PROXY_TABS));
CheckBool(dictionary, "themesSynced", types.Has(syncer::THEMES));
CheckBool(dictionary, "typedUrlsSynced", types.Has(syncer::TYPED_URLS));
CheckBool(dictionary, "wifiCredentialsSynced",
types.Has(syncer::WIFI_CREDENTIALS));
}


Expand Down Expand Up @@ -923,6 +927,7 @@ TEST_F(SyncSetupHandlerTest, ShowSetupSyncEverything) {
CheckBool(dictionary, "extensionsRegistered", true);
CheckBool(dictionary, "passwordsRegistered", true);
CheckBool(dictionary, "preferencesRegistered", true);
CheckBool(dictionary, "wifiCredentialsRegistered", true);
CheckBool(dictionary, "tabsRegistered", true);
CheckBool(dictionary, "themesRegistered", true);
CheckBool(dictionary, "typedUrlsRegistered", true);
Expand Down
1 change: 1 addition & 0 deletions components/sync_driver/model_association_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ static const syncer::ModelType kStartOrder[] = {
syncer::SUPERVISED_USER_SETTINGS,
syncer::SUPERVISED_USER_SHARED_SETTINGS,
syncer::ARTICLES,
syncer::WIFI_CREDENTIALS,
};

COMPILE_ASSERT(arraysize(kStartOrder) ==
Expand Down
1 change: 1 addition & 0 deletions components/sync_driver/pref_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const char kSyncSyncedNotifications[] = "sync.synced_notifications";
const char kSyncTabs[] = "sync.tabs";
const char kSyncThemes[] = "sync.themes";
const char kSyncTypedUrls[] = "sync.typed_urls";
const char kSyncWifiCredentials[] = "sync.wifi_credentials";

// Boolean used by enterprise configuration management in order to lock down
// sync.
Expand Down
1 change: 1 addition & 0 deletions components/sync_driver/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ extern const char kSyncSyncedNotifications[];
extern const char kSyncTabs[];
extern const char kSyncThemes[];
extern const char kSyncTypedUrls[];
extern const char kSyncWifiCredentials[];

extern const char kSyncManaged[];
extern const char kSyncSuppressStart[];
Expand Down
3 changes: 3 additions & 0 deletions components/sync_driver/sync_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ void SyncPrefs::RegisterProfilePrefs(
model_set.Put(syncer::TYPED_URLS);
model_set.Put(syncer::SESSIONS);
model_set.Put(syncer::ARTICLES);
model_set.Put(syncer::WIFI_CREDENTIALS);
registry->RegisterListPref(prefs::kSyncAcknowledgedSyncTypes,
syncer::ModelTypeSetToValue(model_set),
user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
Expand Down Expand Up @@ -343,6 +344,8 @@ const char* SyncPrefs::GetPrefNameForDataType(syncer::ModelType data_type) {
return prefs::kSyncSupervisedUserSharedSettings;
case syncer::DEVICE_INFO:
return prefs::kSyncDeviceInfo;
case syncer::WIFI_CREDENTIALS:
return prefs::kSyncWifiCredentials;
default:
break;
}
Expand Down
3 changes: 2 additions & 1 deletion components/sync_driver/user_selectable_sync_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ enum UserSelectableSyncType {
// TODO(petewil): There was talk of removing this from user selectable sync
// types. Should we?
SYNCED_NOTIFICATIONS = 9,
WIFI_CREDENTIAL = 10,

// The datatypes below are implicitly synced, and are not exposed via user
// selectable checkboxes.
Expand All @@ -57,7 +58,7 @@ enum UserSelectableSyncType {
// SYNCED_NOTIFICATION_APP_INFO

// Number of sync datatypes exposed to the user via checboxes in the UI.
SELECTABLE_DATATYPE_COUNT = 10,
SELECTABLE_DATATYPE_COUNT = 11,
};

} // namespace user_selectable_type
Expand Down
6 changes: 6 additions & 0 deletions sync/internal_api/public/base/model_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#ifndef SYNC_INTERNAL_API_PUBLIC_BASE_MODEL_TYPE_H_
#define SYNC_INTERNAL_API_PUBLIC_BASE_MODEL_TYPE_H_

#include <map>
#include <set>
#include <string>

Expand Down Expand Up @@ -110,6 +111,9 @@ enum ModelType {
ARTICLES,
// App List items
APP_LIST,
// WiFi credentials. Each item contains the information for connecting to one
// WiFi network. This includes, e.g., network name and password.
WIFI_CREDENTIALS,

// ---- Proxy types ----
// Proxy types are excluded from the sync protocol, but are still considered
Expand Down Expand Up @@ -145,6 +149,7 @@ typedef EnumSet<ModelType, FIRST_REAL_MODEL_TYPE, LAST_REAL_MODEL_TYPE>
ModelTypeSet;
typedef EnumSet<ModelType, UNSPECIFIED, LAST_REAL_MODEL_TYPE>
FullModelTypeSet;
typedef std::map<syncer::ModelType, const char*> ModelTypeNameMap;

inline ModelType ModelTypeFromInt(int i) {
DCHECK_GE(i, 0);
Expand Down Expand Up @@ -182,6 +187,7 @@ SYNC_EXPORT ModelTypeSet UserTypes();
// These are the user-selectable data types.
SYNC_EXPORT ModelTypeSet UserSelectableTypes();
SYNC_EXPORT bool IsUserSelectableType(ModelType model_type);
SYNC_EXPORT ModelTypeNameMap GetUserSelectableTypeNameMap();

// This is the subset of UserTypes() that can be encrypted.
SYNC_EXPORT_PRIVATE ModelTypeSet EncryptableUserTypes();
Expand Down
4 changes: 2 additions & 2 deletions sync/internal_api/public/sync_encryption_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ SyncEncryptionHandler::~SyncEncryptionHandler() {}

// Static.
ModelTypeSet SyncEncryptionHandler::SensitiveTypes() {
// It has its own encryption scheme, but we include it anyway.
ModelTypeSet types;
types.Put(PASSWORDS);
types.Put(PASSWORDS); // Has its own encryption, but include it anyway.
types.Put(WIFI_CREDENTIALS);
return types;
}

Expand Down
12 changes: 8 additions & 4 deletions sync/internal_api/sync_encryption_handler_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,8 @@ TEST_F(SyncEncryptionHandlerImplTest, EncryptEverythingExplicit) {
EXPECT_FALSE(encryption_handler()->EncryptEverythingEnabled());
ModelTypeSet encrypted_types =
encryption_handler()->GetEncryptedTypesUnsafe();
EXPECT_TRUE(encrypted_types.Equals(ModelTypeSet(PASSWORDS)));
EXPECT_TRUE(encrypted_types.Equals(
ModelTypeSet(PASSWORDS, WIFI_CREDENTIALS)));

{
WriteTransaction trans(FROM_HERE, user_share());
Expand Down Expand Up @@ -459,7 +460,8 @@ TEST_F(SyncEncryptionHandlerImplTest, EncryptEverythingImplicit) {
EXPECT_FALSE(encryption_handler()->EncryptEverythingEnabled());
ModelTypeSet encrypted_types =
encryption_handler()->GetEncryptedTypesUnsafe();
EXPECT_TRUE(encrypted_types.Equals(ModelTypeSet(PASSWORDS)));
EXPECT_TRUE(encrypted_types.Equals(
ModelTypeSet(PASSWORDS, WIFI_CREDENTIALS)));

{
WriteTransaction trans(FROM_HERE, user_share());
Expand Down Expand Up @@ -503,7 +505,8 @@ TEST_F(SyncEncryptionHandlerImplTest, UnknownSensitiveTypes) {
EXPECT_FALSE(encryption_handler()->EncryptEverythingEnabled());
ModelTypeSet encrypted_types =
encryption_handler()->GetEncryptedTypesUnsafe();
EXPECT_TRUE(encrypted_types.Equals(ModelTypeSet(PASSWORDS)));
EXPECT_TRUE(encrypted_types.Equals(
ModelTypeSet(PASSWORDS, WIFI_CREDENTIALS)));

{
WriteTransaction trans(FROM_HERE, user_share());
Expand All @@ -514,7 +517,8 @@ TEST_F(SyncEncryptionHandlerImplTest, UnknownSensitiveTypes) {

EXPECT_FALSE(encryption_handler()->EncryptEverythingEnabled());
encrypted_types = encryption_handler()->GetEncryptedTypesUnsafe();
EXPECT_TRUE(encrypted_types.Equals(ModelTypeSet(BOOKMARKS, PASSWORDS)));
EXPECT_TRUE(encrypted_types.Equals(
ModelTypeSet(BOOKMARKS, PASSWORDS, WIFI_CREDENTIALS)));
}

// Receive an old nigori with old encryption keys and encrypted types. We should
Expand Down
1 change: 1 addition & 0 deletions sync/protocol/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ proto_library("protocol") {
"theme_specifics.proto",
"typed_url_specifics.proto",
"unique_position.proto",
"wifi_credential_specifics.proto",
]

cc_generator_options = "dllexport_decl=SYNC_PROTO_EXPORT:"
Expand Down
13 changes: 13 additions & 0 deletions sync/protocol/proto_enum_conversions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,19 @@ const char* GetPageTransitionRedirectTypeString(
return "";
}

const char* GetWifiCredentialSecurityClassString(
sync_pb::WifiCredentialSpecifics::SecurityClass security_class) {
ASSERT_ENUM_BOUNDS(sync_pb::WifiCredentialSpecifics, SecurityClass,
SECURITY_CLASS_INVALID, SECURITY_CLASS_PSK);
switch (security_class) {
ENUM_CASE(sync_pb::WifiCredentialSpecifics, SECURITY_CLASS_INVALID);
ENUM_CASE(sync_pb::WifiCredentialSpecifics, SECURITY_CLASS_NONE);
ENUM_CASE(sync_pb::WifiCredentialSpecifics, SECURITY_CLASS_WEP);
ENUM_CASE(sync_pb::WifiCredentialSpecifics, SECURITY_CLASS_PSK);
}
NOTREACHED();
return "";
}
const char* GetUpdatesSourceString(
sync_pb::GetUpdatesCallerInfo::GetUpdatesSource updates_source) {
ASSERT_ENUM_BOUNDS(sync_pb::GetUpdatesCallerInfo, GetUpdatesSource,
Expand Down
3 changes: 3 additions & 0 deletions sync/protocol/proto_enum_conversions.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ SYNC_EXPORT_PRIVATE const char* GetPageTransitionRedirectTypeString(
sync_pb::SyncEnums::PageTransitionRedirectType
redirect_type);

SYNC_EXPORT_PRIVATE const char* GetWifiCredentialSecurityClassString(
sync_pb::WifiCredentialSpecifics::SecurityClass security_class);

SYNC_EXPORT const char* GetUpdatesSourceString(
sync_pb::GetUpdatesCallerInfo::GetUpdatesSource updates_source);

Expand Down
7 changes: 7 additions & 0 deletions sync/protocol/proto_enum_conversions_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ TEST_F(ProtoEnumConversionsTest, GetPageTransitionQualifierString) {
sync_pb::SyncEnums::PageTransitionRedirectType_MAX);
}

TEST_F(ProtoEnumConversionsTest, GetWifiCredentialSecurityClassString) {
TestEnumStringFunction(
GetWifiCredentialSecurityClassString,
sync_pb::WifiCredentialSpecifics::SecurityClass_MIN,
sync_pb::WifiCredentialSpecifics::SecurityClass_MAX);
}

TEST_F(ProtoEnumConversionsTest, GetUpdatesSourceString) {
TestEnumStringFunction(
GetUpdatesSourceString,
Expand Down
10 changes: 10 additions & 0 deletions sync/protocol/proto_value_conversions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,15 @@ base::DictionaryValue* TypedUrlSpecificsToValue(
return value;
}

base::DictionaryValue* WifiCredentialSpecificsToValue(
const sync_pb::WifiCredentialSpecifics& proto) {
base::DictionaryValue* value = new base::DictionaryValue();
SET_BYTES(ssid);
SET_ENUM(security_class, GetWifiCredentialSecurityClassString);
SET_BYTES(passphrase);
return value;
}

base::DictionaryValue* EntitySpecificsToValue(
const sync_pb::EntitySpecifics& specifics) {
base::DictionaryValue* value = new base::DictionaryValue();
Expand Down Expand Up @@ -817,6 +826,7 @@ base::DictionaryValue* EntitySpecificsToValue(
SyncedNotificationAppInfoSpecificsToValue);
SET_FIELD(theme, ThemeSpecificsToValue);
SET_FIELD(typed_url, TypedUrlSpecificsToValue);
SET_FIELD(wifi_credential, WifiCredentialSpecificsToValue);
return value;
}

Expand Down
Loading

0 comments on commit dfad17b

Please sign in to comment.