Fix CHECK that relies on external sync sessions data

Tab sync entities are expected to have a valid |tab_id| field over the
sync protocol, but clients should not crash if the received data is
corrupt, e.g. committed by a buggy client.

Prior to this patch, foreign sessions received in SessionsSyncManager,
processed via UpdateTrackerWithSpecifics(), could trigger a CHECK
failure in SyncedSessionTracker::GetTab().

Bug: 840876
Change-Id: Ie59cfb9c24934695a1b7b6187dbad48451f8c8ea
Reviewed-on: https://chromium-review.googlesource.com/1051233Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557124}

components/sync_sessions/synced_session_tracker.cc

@@ -646,8 +646,8 @@ void UpdateTrackerWithSpecifics(const sync_pb::SessionSpecifics& specifics,
     // for their tabs.
 
     if (!IsValidSessionHeader(specifics.header())) {
-      LOG(WARNING) << "Ignoring session node with invalid header "
-                   << "and tag " << session_tag << ".";
+      DLOG(WARNING) << "Ignoring session node with invalid header "
+                    << "and tag " << session_tag << ".";
       return;
     }
 
@@ -667,6 +667,13 @@ void UpdateTrackerWithSpecifics(const sync_pb::SessionSpecifics& specifics,
   } else if (specifics.has_tab()) {
     const sync_pb::SessionTab& tab_s = specifics.tab();
     SessionID tab_id = SessionID::FromSerializedValue(tab_s.tab_id());
+    if (!tab_id.is_valid()) {
+      DLOG(WARNING) << "Ignoring session tab with invalid tab ID for session "
+                    << "tag " << session_tag << " and node ID "
+                    << specifics.tab_node_id() << ".";
+      return;
+    }
+
     DVLOG(1) << "Populating " << session_tag << "'s tab id " << tab_id
              << " from node " << specifics.tab_node_id();
 

components/sync_sessions/synced_session_tracker_unittest.cc

@@ -888,6 +888,19 @@ TEST_F(SyncedSessionTrackerTest, UpdateTrackerWithHeaderWithDuplicateTabIds) {
               MatchesSyncedSession(kTag, /*window_id_to_tabs=*/{}));
 }
 
+// Verifies that an invalid tab (with invalid ID) is discarded.
+TEST_F(SyncedSessionTrackerTest, UpdateTrackerWithInvalidTab) {
+  const int kInvalidTabId = -1;
+  sync_pb::SessionSpecifics tab;
+  tab.set_session_tag(kTag);
+  tab.mutable_tab()->set_tab_id(kInvalidTabId);
+  UpdateTrackerWithSpecifics(tab, base::Time::Now(), GetTracker());
+
+  EXPECT_THAT(GetTracker()->LookupSessionTab(
+                  kTag, SessionID::FromSerializedValue(kInvalidTabId)),
+              IsNull());
+}
+
 TEST_F(SyncedSessionTrackerTest, UpdateTrackerWithTab) {
   sync_pb::SessionSpecifics tab;
   tab.set_session_tag(kTag);