Propagate device id v2 for existing devices and delete device by devi…

…ce id v2

browser/ui/webui/sync/sync_ui.cc

@@ -203,13 +203,13 @@ void SyncUIDOMHandler::SyncSavedSiteSettings(const base::ListValue* args) {
 
 void SyncUIDOMHandler::DeleteDevice(const base::ListValue* args) {
   DCHECK(sync_service_);
-  int i_device_id = -1;
-  if (!args->GetInteger(0, &i_device_id) || i_device_id == -1) {
-    DCHECK(false) << "[Brave Sync] could not get device id";
+  std::string device_id_v2;
+  if (!args->GetString(0, &device_id_v2)) {
+    DCHECK(false) << "[Brave Sync] could not get device id v2";
     return;
   }
 
-  sync_service_->OnDeleteDevice(std::to_string(i_device_id));
+  sync_service_->OnDeleteDevice(device_id_v2);
 }
 
 void SyncUIDOMHandler::ResetSync(const base::ListValue* args) {

components/brave_sync/brave_profile_sync_service_impl.cc

@@ -283,18 +283,19 @@ void BraveProfileSyncServiceImpl::OnSetupSyncNewToSync(
   brave_sync_prefs_->SetSyncEnabled(true);
 }
 
-void BraveProfileSyncServiceImpl::OnDeleteDevice(const std::string& device_id) {
+void BraveProfileSyncServiceImpl::OnDeleteDevice(
+    const std::string& device_id_v2) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   auto sync_devices = brave_sync_prefs_->GetSyncDevices();
 
-  const SyncDevice* device = sync_devices->GetByDeviceId(device_id);
+  const SyncDevice* device = sync_devices->GetByDeviceIdV2(device_id_v2);
   if (device) {
     const std::string device_name = device->name_;
-    const std::string device_id_v2 = device->device_id_v2_;
+    const std::string device_id = device->device_id_;
     const std::string object_id = device->object_id_;
     SendDeviceSyncRecord(SyncRecord::Action::A_DELETE, device_name, device_id,
                          device_id_v2, object_id);
-    if (device_id == brave_sync_prefs_->GetThisDeviceId()) {
+    if (device_id_v2 == brave_sync_prefs_->GetThisDeviceIdV2()) {
       // Mark state we have sent DELETE for own device and we are going to
       // call ResetSyncInternal() at OnRecordsSent after ensuring we had made
       // a proper attemp to send the record
@@ -315,8 +316,8 @@ void BraveProfileSyncServiceImpl::OnResetSync() {
   } else {
     // We have to send delete record and wait for library deleted response then
     // we can reset it by ResetSyncInternal()
-    const std::string device_id = brave_sync_prefs_->GetThisDeviceId();
-    OnDeleteDevice(device_id);
+    const std::string device_id_v2 = brave_sync_prefs_->GetThisDeviceIdV2();
+    OnDeleteDevice(device_id_v2);
   }
 }
 
@@ -878,25 +879,35 @@ void BraveProfileSyncServiceImpl::SendCreateDevice() {
   brave_sync_prefs_->SetThisDeviceObjectId(object_id);
   std::string device_id = brave_sync_prefs_->GetThisDeviceId();
   std::string device_id_v2 = brave_sync_prefs_->GetThisDeviceIdV2();
-  CHECK(!device_id.empty());
+  DCHECK(!device_id_v2.empty());
 
   SendDeviceSyncRecord(SyncRecord::Action::A_CREATE, device_name, device_id,
                        device_id_v2, object_id);
 }
 
-void BraveProfileSyncServiceImpl::SendUpdateDevice() {
+void BraveProfileSyncServiceImpl::SendDeleteDevice() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  std::string device_id = brave_sync_prefs_->GetThisDeviceId();
   std::string device_name = brave_sync_prefs_->GetThisDeviceName();
+  std::string object_id = brave_sync_prefs_->GetThisDeviceObjectId();
+  std::string device_id = brave_sync_prefs_->GetThisDeviceId();
   std::string device_id_v2 = brave_sync_prefs_->GetThisDeviceIdV2();
-  auto sync_devices = brave_sync_prefs_->GetSyncDevices();
-  // TODO(darkdh): need to get device by object id
-  const SyncDevice* device = sync_devices->GetByDeviceId(device_id);
-  if (device) {
-    std::string object_id = device->object_id_;
+  if (object_id.empty()) {
+    auto sync_devices = brave_sync_prefs_->GetSyncDevices();
+    std::vector<const SyncDevice*> devices =
+      sync_devices->GetByDeviceId(device_id);
+    for (auto* device : devices) {
+      if (device) {
+        object_id = device->object_id_;
+      }
+      SendDeviceSyncRecord(SyncRecord::Action::A_DELETE, device_name, device_id,
+                           device_id_v2, object_id);
+    }
+    DCHECK(!object_id.empty());
+  } else {
+    DCHECK(!device_id_v2.empty());
 
-    SendDeviceSyncRecord(SyncRecord::Action::A_UPDATE, device_name, device_id,
+    SendDeviceSyncRecord(SyncRecord::Action::A_DELETE, device_name, device_id,
                          device_id_v2, object_id);
   }
 }
@@ -917,7 +928,8 @@ void BraveProfileSyncServiceImpl::SendDeviceSyncRecord(
 
 void BraveProfileSyncServiceImpl::OnResolvedPreferences(
     const RecordsList& records) {
-  const std::string this_device_id_v2 = brave_sync_prefs_->GetThisDeviceIdV2();
+  const std::string this_device_object_id =
+    brave_sync_prefs_->GetThisDeviceObjectId();
   bool this_device_deleted = false;
 
   auto sync_devices = brave_sync_prefs_->GetSyncDevices();
@@ -933,13 +945,12 @@ void BraveProfileSyncServiceImpl::OnResolvedPreferences(
           record->action, &actually_merged);
       this_device_deleted =
           this_device_deleted ||
-          (device.deviceIdV2 == this_device_id_v2 &&
+          (record->objectId == this_device_object_id &&
            record->action == SyncRecord::Action::A_DELETE && actually_merged);
     }
   }  // for each device
 
   brave_sync_prefs_->SetSyncDevices(*sync_devices);
-
   if (this_device_deleted) {
     ResetSyncInternal();
   }
@@ -989,8 +1000,11 @@ void BraveProfileSyncServiceImpl::OnPollSyncCycle(GetRecordsCallback cb,
     this_device_created_time_ = base::Time::Now();
   }
   if (send_device_id_v2_update_) {
-    // SendUpdateDevice();
-    OnDeleteDevice(brave_sync_prefs_->GetThisDeviceId());
+    // Because device id might get duplicated and we didn't save object id for
+    // this device so there is no way to send update to propagate device id v2,
+    // We have to delete previous device records by device id and create a new
+    // one.
+    SendDeleteDevice();
     SendCreateDevice();
     send_device_id_v2_update_ = false;
   }

components/brave_sync/brave_profile_sync_service_impl.h

@@ -70,7 +70,7 @@ class BraveProfileSyncServiceImpl
   void OnSetupSyncHaveCode(const std::string& sync_words,
                            const std::string& device_name) override;
   void OnSetupSyncNewToSync(const std::string& device_name) override;
-  void OnDeleteDevice(const std::string& device_id) override;
+  void OnDeleteDevice(const std::string& device_id_v2) override;
   void OnResetSync() override;
   void GetSettingsAndDevices(
       const GetSettingsAndDevicesCallback& callback) override;
@@ -173,7 +173,7 @@ class BraveProfileSyncServiceImpl
                         int max_records);
   void FetchDevices();
   void SendCreateDevice();
-  void SendUpdateDevice();
+  void SendDeleteDevice();
   void SendDeviceSyncRecord(const int action,
                             const std::string& device_name,
                             const std::string& device_id,

components/brave_sync/brave_sync_service.h

@@ -35,7 +35,7 @@ class BraveSyncService {
       const std::string& sync_words,
       const std::string& device_name) = 0;
   virtual void OnSetupSyncNewToSync(const std::string& device_name) = 0;
-  virtual void OnDeleteDevice(const std::string& device_id) = 0;
+  virtual void OnDeleteDevice(const std::string& device_id_v2) = 0;
   virtual void OnResetSync() = 0;
   virtual void GetSettingsAndDevices(
       const GetSettingsAndDevicesCallback& callback) = 0;

components/brave_sync/sync_devices.cc

@@ -184,9 +184,22 @@ SyncDevice* SyncDevices::GetByObjectId(const std::string &object_id) {
   return nullptr;
 }
 
-const SyncDevice* SyncDevices::GetByDeviceId(const std::string &device_id) {
+std::vector<const SyncDevice*>
+SyncDevices::GetByDeviceId(const std::string &device_id) {
+  std::vector<const SyncDevice*> devices;
   for (const auto& device : devices_) {
     if (device.device_id_ == device_id) {
+      devices.push_back(&device);
+    }
+  }
+
+  return devices;
+}
+
+const SyncDevice*
+SyncDevices::GetByDeviceIdV2(const std::string &device_id_v2) {
+  for (const auto& device : devices_) {
+    if (device.device_id_v2_ == device_id_v2) {
       return &device;
     }
   }

components/brave_sync/sync_devices.h

@@ -48,7 +48,9 @@ class SyncDevices {
    void FromJson(const std::string &str_json);
    void Merge(const SyncDevice& device, int action, bool* actually_merged);
 
-   const SyncDevice* GetByDeviceId(const std::string& device_id);
+   // deprecated. only used for migration and backward compatibility
+   std::vector<const SyncDevice*> GetByDeviceId(const std::string& device_id);
+   const SyncDevice* GetByDeviceIdV2(const std::string& device_id_v2);
    SyncDevice* GetByObjectId(const std::string& object_id);
    void DeleteByObjectId(const std::string& object_id);
 };

components/brave_sync/ui/actions/sync_actions.ts

@@ -56,11 +56,11 @@ export const onGenerateQRCodeImageSource = (imageSource: string) => {
 
 /**
  * Dispatches a message telling the back-end that the user removed a given device
- * @param {number} id - The device ID
+ * @param {number} id - The device ID V2
  * @param {string} deviceName - The device name
  */
-export const onRemoveDevice = (id: number, deviceName: string) => {
-  return action(types.SYNC_ON_REMOVE_DEVICE, { id, deviceName })
+export const onRemoveDevice = (idv2: number, deviceName: string) => {
+  return action(types.SYNC_ON_REMOVE_DEVICE, { idv2, deviceName })
 }
 
 /**

components/brave_sync/ui/containers/enabledContent.tsx

@@ -90,6 +90,7 @@ export default class SyncEnabledContent extends React.PureComponent<Props, State
             content: (
               <TableRowRemoveButton
                 data-id={device.id}
+                data-idv2={device.id_v2}
                 data-name={device.name}
                 onClick={this.onClickRemoveDeviceButton}
               >
@@ -145,7 +146,7 @@ export default class SyncEnabledContent extends React.PureComponent<Props, State
     const target = event.currentTarget as HTMLButtonElement
     this.setState({
       deviceToRemoveName: target.dataset.name,
-      deviceToRemoveId: target.dataset.id,
+      deviceToRemoveId: target.dataset.idv2,
       removeDevice: true
     })
   }
@@ -223,7 +224,7 @@ export default class SyncEnabledContent extends React.PureComponent<Props, State
                 <RemoveDeviceModal
                   syncData={syncData}
                   deviceName={deviceToRemoveName}
-                  deviceId={deviceToRemoveId}
+                  deviceIdV2={deviceToRemoveId}
                   actions={actions}
                   onClose={this.onClickCancelRemoveDeviceButton}
                 />

components/brave_sync/ui/containers/modals/removeDevice.tsx

@@ -28,7 +28,7 @@ interface Props {
   onClose: (event?: React.MouseEvent<HTMLButtonElement>) => void
   actions: any
   deviceName: string | undefined
-  deviceId: string | undefined
+  deviceIdV2: string | undefined
 }
 
 interface State {
@@ -59,13 +59,13 @@ export default class RemoveMainDeviceModal extends React.PureComponent<Props, St
   }
 
   onClickConfirmRemoveDeviceButton = () => {
-    const { deviceName, deviceId } = this.props
-    this.props.actions.onRemoveDevice(Number(deviceId), deviceName)
+    const { deviceName, deviceIdV2 } = this.props
+    this.props.actions.onRemoveDevice(deviceIdV2, deviceName)
     this.setState({ willRemoveDevice: true })
   }
 
   render () {
-    const { syncData, deviceName, deviceId } = this.props
+    const { syncData, deviceName, deviceIdV2 } = this.props
     const { willRemoveDevice } = this.state
 
     return (
@@ -75,7 +75,7 @@ export default class RemoveMainDeviceModal extends React.PureComponent<Props, St
         </ModalHeader>
         <ModalContent>
           {
-            deviceId === syncData.thisDeviceId
+            deviceIdV2 === syncData.thisDeviceIdV2
             ? <div>
                 <Paragraph>{getLocale('thisDeviceRemovalDescription')}</Paragraph>
                 <Paragraph>{getLocale('joinSyncChain')}</Paragraph>

components/brave_sync/ui/reducers/sync_reducer.ts

@@ -92,10 +92,10 @@ const syncReducer: Reducer<Sync.State | undefined> = (state: Sync.State | undefi
       break
 
     case types.SYNC_ON_REMOVE_DEVICE:
-      if (typeof payload.id === 'undefined' || typeof payload.deviceName === 'undefined') {
+      if (typeof payload.idv2 === 'undefined' || typeof payload.deviceName === 'undefined') {
         break
       }
-      chrome.send('deleteDevice', [payload.id])
+      chrome.send('deleteDevice', [payload.idv2])
       break
 
     case types.SYNC_BOOKMARKS: