Remove sync_sessions::SyncedSession::DeviceType

We use the proto enum directly in most places, and leave Android code
unchanged for now (we may want to consider exposing proto lite to
Android code in future CLs, but this particular enum doesn't seem to
justify it).

Bug: 681921
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ia3e030a6058a3f7e4e6feb1800fe618ae435ffb4
Reviewed-on: https://chromium-review.googlesource.com/921283Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Nicolas Zea <zea@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Nicolas Dossou-Gbété <dgn@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537635}

chrome/android/java/src/org/chromium/chrome/browser/ntp/ForeignSessionHelper.java

@@ -36,7 +36,7 @@ class ForeignSessionHelper {
      * Represents synced foreign session.
      */
     static class ForeignSession {
-        // Please keep in sync with synced_session.h
+        // Please keep in sync with components/sync/protocol/sync_enums.proto.
         static final int DEVICE_TYPE_UNSET = 0;
         static final int DEVICE_TYPE_WIN = 1;
         static final int DEVICE_TYPE_MACOSX = 2;

chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc

@@ -588,24 +588,24 @@ void RecentTabsSubMenuModel::BuildOtherDevicesTabItem(
 
 void RecentTabsSubMenuModel::AddDeviceFavicon(
     int index_in_menu,
-    sync_sessions::SyncedSession::DeviceType device_type) {
+    sync_pb::SyncEnums::DeviceType device_type) {
 #if defined(OS_MACOSX)
   int favicon_id = -1;
   switch (device_type) {
-    case sync_sessions::SyncedSession::TYPE_PHONE:
+    case sync_pb::SyncEnums::TYPE_PHONE:
       favicon_id = IDR_PHONE_FAVICON;
       break;
 
-    case sync_sessions::SyncedSession::TYPE_TABLET:
+    case sync_pb::SyncEnums::TYPE_TABLET:
       favicon_id = IDR_TABLET_FAVICON;
       break;
 
-    case sync_sessions::SyncedSession::TYPE_CHROMEOS:
-    case sync_sessions::SyncedSession::TYPE_WIN:
-    case sync_sessions::SyncedSession::TYPE_MACOSX:
-    case sync_sessions::SyncedSession::TYPE_LINUX:
-    case sync_sessions::SyncedSession::TYPE_OTHER:
-    case sync_sessions::SyncedSession::TYPE_UNSET:
+    case sync_pb::SyncEnums::TYPE_CROS:
+    case sync_pb::SyncEnums::TYPE_WIN:
+    case sync_pb::SyncEnums::TYPE_MAC:
+    case sync_pb::SyncEnums::TYPE_LINUX:
+    case sync_pb::SyncEnums::TYPE_OTHER:
+    case sync_pb::SyncEnums::TYPE_UNSET:
       favicon_id = IDR_LAPTOP_FAVICON;
       break;
   }
@@ -615,20 +615,20 @@ void RecentTabsSubMenuModel::AddDeviceFavicon(
 #else
   const gfx::VectorIcon* favicon = nullptr;
   switch (device_type) {
-    case sync_sessions::SyncedSession::TYPE_PHONE:
+    case sync_pb::SyncEnums::TYPE_PHONE:
       favicon = &kSmartphoneIcon;
       break;
 
-    case sync_sessions::SyncedSession::TYPE_TABLET:
+    case sync_pb::SyncEnums::TYPE_TABLET:
       favicon = &kTabletIcon;
       break;
 
-    case sync_sessions::SyncedSession::TYPE_CHROMEOS:
-    case sync_sessions::SyncedSession::TYPE_WIN:
-    case sync_sessions::SyncedSession::TYPE_MACOSX:
-    case sync_sessions::SyncedSession::TYPE_LINUX:
-    case sync_sessions::SyncedSession::TYPE_OTHER:
-    case sync_sessions::SyncedSession::TYPE_UNSET:
+    case sync_pb::SyncEnums::TYPE_CROS:
+    case sync_pb::SyncEnums::TYPE_WIN:
+    case sync_pb::SyncEnums::TYPE_MAC:
+    case sync_pb::SyncEnums::TYPE_LINUX:
+    case sync_pb::SyncEnums::TYPE_OTHER:
+    case sync_pb::SyncEnums::TYPE_UNSET:
       favicon = &kLaptopIcon;
       break;
   }

chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.h

@@ -117,7 +117,7 @@ class RecentTabsSubMenuModel : public ui::SimpleMenuModel,
 
   // Add the favicon for the device section header.
   void AddDeviceFavicon(int index_in_menu,
-                        sync_sessions::SyncedSession::DeviceType device_type);
+                        sync_pb::SyncEnums::DeviceType device_type);
 
   // Add the favicon for a local or other devices' tab asynchronously,
   // OnFaviconDataAvailable() will be invoked when the favicon is ready.

chrome/browser/ui/webui/foreign_session_handler.cc

@@ -45,6 +45,31 @@ namespace {
 // Maximum number of sessions we're going to display on the NTP
 const size_t kMaxSessionsToShow = 10;
 
+// Converts the DeviceType enum value to a string. This is used
+// in the NTP handler for foreign sessions for matching session
+// types to an icon style.
+std::string DeviceTypeToString(sync_pb::SyncEnums::DeviceType device_type) {
+  switch (device_type) {
+    case sync_pb::SyncEnums::TYPE_UNSET:
+      break;
+    case sync_pb::SyncEnums::TYPE_WIN:
+      return "win";
+    case sync_pb::SyncEnums::TYPE_MAC:
+      return "macosx";
+    case sync_pb::SyncEnums::TYPE_LINUX:
+      return "linux";
+    case sync_pb::SyncEnums::TYPE_CROS:
+      return "chromeos";
+    case sync_pb::SyncEnums::TYPE_OTHER:
+      return "other";
+    case sync_pb::SyncEnums::TYPE_PHONE:
+      return "phone";
+    case sync_pb::SyncEnums::TYPE_TABLET:
+      return "tablet";
+  }
+  return std::string();
+}
+
 // Helper method to create JSON compatible objects from Session objects.
 std::unique_ptr<base::DictionaryValue> SessionTabToValue(
     const ::sessions::SessionTab& tab) {
@@ -284,7 +309,8 @@ void ForeignSessionHandler::HandleGetForeignSessions(
       // remove any keys here.
       session_data->SetString("tag", session_tag);
       session_data->SetString("name", session->session_name);
-      session_data->SetString("deviceType", session->DeviceTypeAsString());
+      session_data->SetString("deviceType",
+                              DeviceTypeToString(session->device_type));
       session_data->SetString("modifiedTime",
                               FormatSessionTime(session->modified_time));
       session_data->SetDouble("timestamp", session->modified_time.ToJsTime());

components/sync/protocol/proto_enum_conversions.cc

@@ -160,8 +160,9 @@ const char* ProtoEnumToString(sync_pb::SyncEnums::Action action) {
 }
 
 const char* ProtoEnumToString(sync_pb::SyncEnums::DeviceType device_type) {
-  ASSERT_ENUM_BOUNDS(sync_pb::SyncEnums, DeviceType, TYPE_WIN, TYPE_TABLET);
+  ASSERT_ENUM_BOUNDS(sync_pb::SyncEnums, DeviceType, TYPE_UNSET, TYPE_TABLET);
   switch (device_type) {
+    ENUM_CASE(sync_pb::SyncEnums, TYPE_UNSET);
     ENUM_CASE(sync_pb::SyncEnums, TYPE_WIN);
     ENUM_CASE(sync_pb::SyncEnums, TYPE_MAC);
     ENUM_CASE(sync_pb::SyncEnums, TYPE_LINUX);

components/sync/protocol/sync_enums.proto

@@ -120,7 +120,9 @@ message SyncEnums {
     UNKNOWN_ACTION = 5;              // This is the default.
   }
 
+  // Please keep in sync with chrome/android/java/.../ForeignSessionHelper.java
   enum DeviceType {
+    TYPE_UNSET = 0;
     TYPE_WIN = 1;
     TYPE_MAC = 2;
     TYPE_LINUX = 3;

components/sync_sessions/session_sync_test_helper.cc

@@ -51,7 +51,7 @@ void SessionSyncTestHelper::VerifySyncedSession(
     const std::vector<std::vector<SessionID::id_type>>& windows,
     const SyncedSession& session) {
   ASSERT_EQ(tag, session.session_tag);
-  ASSERT_EQ(SyncedSession::TYPE_LINUX, session.device_type);
+  ASSERT_EQ(sync_pb::SyncEnums_DeviceType_TYPE_LINUX, session.device_type);
   ASSERT_EQ(kClientName, session.session_name);
   ASSERT_EQ(windows.size(), session.windows.size());
 

components/sync_sessions/sessions_sync_manager.cc

@@ -110,27 +110,6 @@ bool ShouldSyncTabId(SessionID::id_type tab_id) {
   return true;
 }
 
-SyncedSession::DeviceType ProtoDeviceTypeToSyncedSessionDeviceType(
-    sync_pb::SyncEnums::DeviceType proto_device_type) {
-  switch (proto_device_type) {
-    case sync_pb::SyncEnums_DeviceType_TYPE_WIN:
-      return SyncedSession::TYPE_WIN;
-    case sync_pb::SyncEnums_DeviceType_TYPE_MAC:
-      return SyncedSession::TYPE_MACOSX;
-    case sync_pb::SyncEnums_DeviceType_TYPE_LINUX:
-      return SyncedSession::TYPE_LINUX;
-    case sync_pb::SyncEnums_DeviceType_TYPE_CROS:
-      return SyncedSession::TYPE_CHROMEOS;
-    case sync_pb::SyncEnums_DeviceType_TYPE_PHONE:
-      return SyncedSession::TYPE_PHONE;
-    case sync_pb::SyncEnums_DeviceType_TYPE_TABLET:
-      return SyncedSession::TYPE_TABLET;
-    case sync_pb::SyncEnums_DeviceType_TYPE_OTHER:
-      return SyncedSession::TYPE_OTHER;
-  }
-  return SyncedSession::TYPE_OTHER;
-}
-
 bool IsWindowSyncable(const SyncedWindowDelegate& window_delegate) {
   return window_delegate.ShouldSync() && window_delegate.GetTabCount() &&
          window_delegate.HasWindow();
@@ -266,8 +245,7 @@ void SessionsSyncManager::AssociateWindows(
   SyncedSession* current_session =
       session_tracker_.GetSession(current_machine_tag());
   current_session->session_name = current_session_name_;
-  current_session->device_type =
-      ProtoDeviceTypeToSyncedSessionDeviceType(current_device_type_);
+  current_session->device_type = current_device_type_;
   current_session->session_tag = current_machine_tag();
 
   SyncedWindowDelegatesGetter::SyncedWindowDelegateMap windows =
@@ -1056,8 +1034,7 @@ void SessionsSyncManager::PopulateSyncedSessionFromSpecifics(
   if (header_specifics.has_client_name())
     synced_session->session_name = header_specifics.client_name();
   if (header_specifics.has_device_type()) {
-    synced_session->device_type = ProtoDeviceTypeToSyncedSessionDeviceType(
-        header_specifics.device_type());
+    synced_session->device_type = header_specifics.device_type();
   }
   synced_session->modified_time =
       std::max(mtime, synced_session->modified_time);

components/sync_sessions/synced_session.cc

@@ -23,7 +23,7 @@ sync_pb::SessionWindow SyncedSessionWindow::ToSessionWindowProto() const {
 }
 
 SyncedSession::SyncedSession()
-    : session_tag("invalid"), device_type(TYPE_UNSET) {}
+    : session_tag("invalid"), device_type(sync_pb::SyncEnums::TYPE_UNSET) {}
 
 SyncedSession::~SyncedSession() {}
 
@@ -34,31 +34,7 @@ sync_pb::SessionHeader SyncedSession::ToSessionHeaderProto() const {
     w->CopyFrom(window_pair.second->ToSessionWindowProto());
   }
   header.set_client_name(session_name);
-  switch (device_type) {
-    case SyncedSession::TYPE_WIN:
-      header.set_device_type(sync_pb::SyncEnums_DeviceType_TYPE_WIN);
-      break;
-    case SyncedSession::TYPE_MACOSX:
-      header.set_device_type(sync_pb::SyncEnums_DeviceType_TYPE_MAC);
-      break;
-    case SyncedSession::TYPE_LINUX:
-      header.set_device_type(sync_pb::SyncEnums_DeviceType_TYPE_LINUX);
-      break;
-    case SyncedSession::TYPE_CHROMEOS:
-      header.set_device_type(sync_pb::SyncEnums_DeviceType_TYPE_CROS);
-      break;
-    case SyncedSession::TYPE_PHONE:
-      header.set_device_type(sync_pb::SyncEnums_DeviceType_TYPE_PHONE);
-      break;
-    case SyncedSession::TYPE_TABLET:
-      header.set_device_type(sync_pb::SyncEnums_DeviceType_TYPE_TABLET);
-      break;
-    case SyncedSession::TYPE_OTHER:
-    // Intentionally fall-through
-    default:
-      header.set_device_type(sync_pb::SyncEnums_DeviceType_TYPE_OTHER);
-      break;
-  }
+  header.set_device_type(device_type);
   return header;
 }
 

components/sync_sessions/synced_session.h

@@ -15,6 +15,7 @@
 #include "components/sessions/core/session_id.h"
 #include "components/sessions/core/session_types.h"
 #include "components/sync/protocol/session_specifics.pb.h"
+#include "components/sync/protocol/sync_enums.pb.h"
 
 namespace sync_sessions {
 
@@ -40,19 +41,6 @@ struct SyncedSessionWindow {
 // list of windows along with a unique session identifer (tag) and meta-data
 // about the device being synced.
 struct SyncedSession {
-  // The type of device.
-  // Please keep in sync with ForeignSessionHelper.java
-  enum DeviceType {
-    TYPE_UNSET = 0,
-    TYPE_WIN = 1,
-    TYPE_MACOSX = 2,
-    TYPE_LINUX = 3,
-    TYPE_CHROMEOS = 4,
-    TYPE_OTHER = 5,
-    TYPE_PHONE = 6,
-    TYPE_TABLET = 7
-  };
-
   SyncedSession();
   ~SyncedSession();
 
@@ -62,7 +50,7 @@ struct SyncedSession {
   std::string session_name;
 
   // Type of device this session is from.
-  DeviceType device_type;
+  sync_pb::SyncEnums::DeviceType device_type;
 
   // Last time this session was modified remotely. This is the max of the header
   // and all children tab mtimes.
@@ -82,30 +70,6 @@ struct SyncedSession {
   // SyncedSessionTracker::GetTabImpl for a concrete example of id reuse.
   std::set<int> tab_node_ids;
 
-  // Converts the DeviceType enum value to a string. This is used
-  // in the NTP handler for foreign sessions for matching session
-  // types to an icon style.
-  std::string DeviceTypeAsString() const {
-    switch (device_type) {
-      case SyncedSession::TYPE_WIN:
-        return "win";
-      case SyncedSession::TYPE_MACOSX:
-        return "macosx";
-      case SyncedSession::TYPE_LINUX:
-        return "linux";
-      case SyncedSession::TYPE_CHROMEOS:
-        return "chromeos";
-      case SyncedSession::TYPE_OTHER:
-        return "other";
-      case SyncedSession::TYPE_PHONE:
-        return "phone";
-      case SyncedSession::TYPE_TABLET:
-        return "tablet";
-      default:
-        return std::string();
-    }
-  }
-
   // Convert this object to its protocol buffer equivalent. Shallow conversion,
   // does not create SessionTab protobufs.
   sync_pb::SessionHeader ToSessionHeaderProto() const;

components/sync_sessions/synced_session_tracker.cc

@@ -160,7 +160,8 @@ bool SyncedSessionTracker::DeleteForeignSession(
   if (iter != synced_session_map_.end()) {
     // An implicitly created session that has children tabs but no header node
     // will have never had the device_type changed from unset.
-    header_existed = iter->second->device_type != SyncedSession::TYPE_UNSET;
+    header_existed =
+        iter->second->device_type != sync_pb::SyncEnums::TYPE_UNSET;
     // SyncedSession's destructor will trigger deletion of windows which will in
     // turn trigger the deletion of tabs. This doesn't affect the convenience
     // maps.

components/sync_sessions/synced_session_tracker_unittest.cc

@@ -225,7 +225,7 @@ TEST_F(SyncedSessionTrackerTest, Complex) {
   SyncedSession* session = GetTracker()->GetSession(kTag);
   SyncedSession* session2 = GetTracker()->GetSession(kTag2);
   SyncedSession* session3 = GetTracker()->GetSession(kTag3);
-  session3->device_type = SyncedSession::TYPE_OTHER;
+  session3->device_type = sync_pb::SyncEnums_DeviceType_TYPE_LINUX;
   ASSERT_EQ(3U, GetTracker()->num_synced_sessions());
 
   ASSERT_TRUE(session);

ios/chrome/browser/ui/ntp/recent_tabs/synced_sessions.h

@@ -60,7 +60,7 @@ class DistantSession {
   std::string tag;
   std::string name;
   base::Time modified_time;
-  sync_sessions::SyncedSession::DeviceType device_type;
+  sync_pb::SyncEnums::DeviceType device_type;
   std::vector<std::unique_ptr<DistantTab>> tabs;
 
   DISALLOW_COPY_AND_ASSIGN(DistantSession);

ios/chrome/browser/ui/ntp/recent_tabs/views/session_section_header_view.mm

@@ -118,10 +118,10 @@ const int kRelativeTimeMaxHours = 4;
     (synced_sessions::DistantSession const*)distantSession {
   NSString* imageName = nil;
   switch (distantSession->device_type) {
-    case sync_sessions::SyncedSession::TYPE_PHONE:
+    case sync_pb::SyncEnums::TYPE_PHONE:
       imageName = @"ntp_opentabs_phone";
       break;
-    case sync_sessions::SyncedSession::TYPE_TABLET:
+    case sync_pb::SyncEnums::TYPE_TABLET:
       imageName = @"ntp_opentabs_tablet";
       break;
     default:

ios/chrome/browser/ui/tab_switcher/tab_switcher_controller.mm

@@ -1142,8 +1142,8 @@ enum class SnapshotViewOption {
     } else {
       index -= kHeaderDistantSessionIndexOffset;
 
-      sync_sessions::SyncedSession::DeviceType deviceType =
-          sync_sessions::SyncedSession::TYPE_UNSET;
+      sync_pb::SyncEnums::DeviceType deviceType =
+          sync_pb::SyncEnums::TYPE_UNSET;
       NSString* cellTitle = nil;
 
       if (index < _controllersOfDistantSessions.count) {
@@ -1156,10 +1156,10 @@ enum class SnapshotViewOption {
       }
       TabSwitcherSessionCellType cellType;
       switch (deviceType) {
-        case sync_sessions::SyncedSession::TYPE_PHONE:
+        case sync_pb::SyncEnums::TYPE_PHONE:
           cellType = kPhoneRemoteSessionCell;
           break;
-        case sync_sessions::SyncedSession::TYPE_TABLET:
+        case sync_pb::SyncEnums::TYPE_TABLET:
           cellType = kTabletRemoteSessionCell;
           break;
         default: