[Sync] Introduce SyncedSessionWindow type.

components/sync_sessions/synced_session.h

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_SYNC_SESSIONS_SYNCED_SESSION_H_
 #define COMPONENTS_SYNC_SESSIONS_SYNCED_SESSION_H_
 
 #include <map>
 #include <memory>
 #include <set>
 #include <string>
 
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "components/sessions/core/session_id.h"
 #include "components/sessions/core/session_types.h"
 #include "components/sync/protocol/session_specifics.pb.h"
 
-namespace sessions {
-struct SessionWindow;
-}
+namespace sync_sessions {
 
-namespace sync_sessions {
+// A Sync wrapper for a SessionWindow.
+struct SyncedSessionWindow {
+  SyncedSessionWindow();
+  ~SyncedSessionWindow();
+
+  // Convert this object into its sync protocol buffer equivalent.
+  sync_pb::SessionWindow ToSyncSessionWindowPB() const;

[skym, 2017/03/28 04:48:55]

The "PB" looks really ugly to me, I'd prefer something like
ToSyncProtoSessionWindow(), but I'm fine with whatever you think is best.

[Nicolas Zea, 2017/03/29 19:01:33]

Switched to ToSessionWindowProto

+
+  // Type of the window. See session_specifics.proto.
+  sync_pb::SessionWindow::BrowserType window_type;
+
+  // The SessionWindow this object wraps.
+  sessions::SessionWindow wrapped_window;

[skym, 2017/03/28 04:48:55]

Does it really make sense to hang onto a SessionWindow indefinitely? It seems
like there are quite a few fields that we don't care about. workspace, bounds,
show_state, app_name, etc. Especially for foreign sessions, having a
wrapped_window is kind of lie. What do you think of holding onto our members
directly here, and having another ToNativeSessionWindow() accessor that creates
a sessions::SessionWindow on demand?

What if we remove all concepts of "wrapping in SessionsSyncManager and friends
and they simply had its own first class models that held data in the way that
was the most convenient to them. I feel like adding .wrapped_window. is going
the wrong way here.

[Patrick Noland, 2017/03/28 20:42:00]

It seems like many accesses of wrapped_window just want the tabs; would it make
sense to add an accessor to insulate these callers from the fact that there's a
wrapped window? 

[Nicolas Zea, 2017/03/29 19:01:33]

I do think it probably makes sense to move away from sessions::SessionWindow
entirely at some point, but I think that's outside of the scope of this patch
(as discussed offline). I think that will eventually also address Patrick's
concern about callers not caring about whether the window is wrapped or not.

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SyncedSessionWindow);
+};
 
 // Defines a synced session for use by session sync. A synced session is a
 // 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,
@@ skipping 14 matching lines @@
   std::string session_name;
 
   // Type of device this session is from.
   DeviceType device_type;
 
   // Last time this session was modified remotely. This is the max of the header
   // and all children tab mtimes.
   base::Time modified_time;
 
   // Map of windows that make up this session.
-  std::map<SessionID::id_type, std::unique_ptr<sessions::SessionWindow>>
-      windows;
+  std::map<SessionID::id_type, std::unique_ptr<SyncedSessionWindow>> windows;
 
   // A tab node id is part of the identifier for the sync tab objects. Tab node
   // ids are not used for interacting with the model/browser tabs. However, when
   // when we want to delete a foreign session, we use these values to inform
   // sync which tabs to delete. We are extracting these tab node ids from
   // individual session (tab, not header) specifics, but store them here in the
   // SyncedSession during runtime. We do this because tab node ids may be reused
   // for different tabs, and tracking which tab id is currently associated with
   // each tab node id is both difficult and unnecessary. See comments at
   // SyncedSessionTracker::GetTabImpl for a concrete example of id reuse.
@@ skipping 18 matching lines @@
         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 ToSessionHeader() const;
+  sync_pb::SessionHeader ToSyncSessionHeaderPB() const;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SyncedSession);
 };
 
 }  // namespace sync_sessions
 
 #endif  // COMPONENTS_SYNC_SESSIONS_SYNCED_SESSION_H_