summaryrefslogtreecommitdiff
path: root/chromium/components/sync_sessions
diff options
context:
space:
mode:
authorAllan Sandfeld Jensen <allan.jensen@qt.io>2018-01-31 16:33:43 +0100
committerAllan Sandfeld Jensen <allan.jensen@qt.io>2018-02-06 16:33:22 +0000
commitda51f56cc21233c2d30f0fe0d171727c3102b2e0 (patch)
tree4e579ab70ce4b19bee7984237f3ce05a96d59d83 /chromium/components/sync_sessions
parentc8c2d1901aec01e934adf561a9fdf0cc776cdef8 (diff)
downloadqtwebengine-chromium-da51f56cc21233c2d30f0fe0d171727c3102b2e0.tar.gz
BASELINE: Update Chromium to 65.0.3525.40
Also imports missing submodules Change-Id: I36901b7c6a325cda3d2c10cedb2186c25af3b79b Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
Diffstat (limited to 'chromium/components/sync_sessions')
-rw-r--r--chromium/components/sync_sessions/revisit/offset_tab_matcher.cc6
-rw-r--r--chromium/components/sync_sessions/sessions_sync_manager.cc72
-rw-r--r--chromium/components/sync_sessions/sessions_sync_manager.h13
-rw-r--r--chromium/components/sync_sessions/sessions_sync_manager_unittest.cc89
-rw-r--r--chromium/components/sync_sessions/synced_session_tracker.cc5
-rw-r--r--chromium/components/sync_sessions/synced_session_tracker.h2
-rw-r--r--chromium/components/sync_sessions/tab_node_pool.cc2
7 files changed, 111 insertions, 78 deletions
diff --git a/chromium/components/sync_sessions/revisit/offset_tab_matcher.cc b/chromium/components/sync_sessions/revisit/offset_tab_matcher.cc
index 5ca14343284..545f2ef0152 100644
--- a/chromium/components/sync_sessions/revisit/offset_tab_matcher.cc
+++ b/chromium/components/sync_sessions/revisit/offset_tab_matcher.cc
@@ -8,8 +8,8 @@
#include <algorithm>
+#include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h"
-#include "base/metrics/sparse_histogram.h"
#include "components/sessions/core/serialized_navigation_entry.h"
#include "components/sessions/core/session_types.h"
@@ -65,8 +65,8 @@ void OffsetTabMatcher::Emit(
// sparse we could end up with a very large output space across many
// clients. So we clamp on a resonable bound that's larger than we expect to
// be sure no unexpected data causes problems.
- UMA_HISTOGRAM_SPARSE_SLOWLY("Sync.PageRevisitNavigationMatchOffset",
- Clamp(best_offset_, -kMaxOffset, kMaxOffset));
+ base::UmaHistogramSparse("Sync.PageRevisitNavigationMatchOffset",
+ Clamp(best_offset_, -kMaxOffset, kMaxOffset));
REVISIT_HISTOGRAM_AGE("Sync.PageRevisitNavigationMatchAge",
best_tab_->timestamp);
UMA_HISTOGRAM_ENUMERATION("Sync.PageRevisitNavigationMatchTransition",
diff --git a/chromium/components/sync_sessions/sessions_sync_manager.cc b/chromium/components/sync_sessions/sessions_sync_manager.cc
index efa9b68cb85..9972d4725cf 100644
--- a/chromium/components/sync_sessions/sessions_sync_manager.cc
+++ b/chromium/components/sync_sessions/sessions_sync_manager.cc
@@ -79,7 +79,8 @@ bool SessionsRecencyComparator(const SyncedSession* s1,
}
std::string TabNodeIdToTag(const std::string& machine_tag, int tab_node_id) {
- CHECK_GT(tab_node_id, TabNodePool::kInvalidTabNodeID) << "crbug.com/673618";
+ CHECK_GT(tab_node_id, TabNodePool::kInvalidTabNodeID)
+ << "https://crbug.com/639009";
return base::StringPrintf("%s %d", machine_tag.c_str(), tab_node_id);
}
@@ -251,7 +252,7 @@ syncer::SyncMergeResult SessionsSyncManager::MergeDataAndStartSyncing(
#endif
// Check if anything has changed on the local client side.
- AssociateWindows(RELOAD_TABS, &new_changes);
+ AssociateWindows(RELOAD_TABS, ScanForTabbedWindow(), &new_changes);
local_tab_pool_out_of_sync_ = false;
merge_result.set_error(
@@ -263,6 +264,7 @@ syncer::SyncMergeResult SessionsSyncManager::MergeDataAndStartSyncing(
void SessionsSyncManager::AssociateWindows(
ReloadTabsOption option,
+ bool has_tabbed_window,
syncer::SyncChangeList* change_output) {
// Note that |current_session| is a pointer owned by |session_tracker_|.
// |session_tracker_| will continue to update |current_session| under
@@ -280,18 +282,10 @@ void SessionsSyncManager::AssociateWindows(
SyncedWindowDelegatesGetter::SyncedWindowDelegateMap windows =
synced_window_delegates_getter()->GetSyncedWindowDelegates();
- // On Android, it's possible to not have any tabbed windows if this is a cold
- // start triggered for a custom tab. In that case, the previous session must
- // be restored, otherwise it will be lost. On the other hand, if there is at
- // least one tabbed window open, it's safe to overwrite the previous session
- // entirely. See crbug.com/639009 for more info.
- bool found_tabbed_window = false;
- for (auto& window_iter_pair : windows) {
- if (window_iter_pair.second->IsTypeTabbed())
- found_tabbed_window = true;
- }
-
- if (found_tabbed_window) {
+ // Without native data, we need be careful not to obliterate any old
+ // information, while at the same time handling updated tab ids. See
+ // https://crbug.com/639009 for more info.
+ if (has_tabbed_window) {
// Just reset the session tracking. No need to worry about the previous
// session; the current tabbed windows are now the source of truth.
session_tracker_.ResetSessionTracking(current_machine_tag());
@@ -326,6 +320,9 @@ void SessionsSyncManager::AssociateWindows(
}
}
+ // TODO(skym): Scan for duplicate sync ids and remove,
+ // https://crbug.com/639009.
+
for (auto& window_iter_pair : windows) {
const SyncedWindowDelegate* window_delegate = window_iter_pair.second;
if (option == RELOAD_TABS) {
@@ -340,7 +337,6 @@ void SessionsSyncManager::AssociateWindows(
// about to be deleted, so we ignore it.
if (window_delegate->ShouldSync() && window_delegate->GetTabCount() &&
window_delegate->HasWindow()) {
- sync_pb::SessionWindow window_s;
SessionID::id_type window_id = window_delegate->GetSessionId();
DVLOG(1) << "Associating window " << window_id << " with "
<< window_delegate->GetTabCount() << " tabs.";
@@ -372,7 +368,7 @@ void SessionsSyncManager::AssociateWindows(
DVLOG(1) << "Placeholder tab " << tab_id << " has no sync id.";
}
} else if (RELOAD_TABS == option) {
- AssociateTab(synced_tab, change_output);
+ AssociateTab(synced_tab, has_tabbed_window, change_output);
}
// If the tab was syncable, it would have been added to the tracker
@@ -432,6 +428,7 @@ void SessionsSyncManager::AssociateWindows(
}
void SessionsSyncManager::AssociateTab(SyncedTabDelegate* const tab_delegate,
+ bool has_tabbed_window,
syncer::SyncChangeList* change_output) {
DCHECK(!tab_delegate->IsPlaceholderTab());
@@ -457,11 +454,20 @@ void SessionsSyncManager::AssociateTab(SyncedTabDelegate* const tab_delegate,
if (session_tracker_.IsLocalTabNodeAssociated(tab_delegate->GetSyncId())) {
tab_node_id = tab_delegate->GetSyncId();
session_tracker_.ReassociateLocalTab(tab_node_id, tab_id);
- } else {
+ } else if (has_tabbed_window) {
existing_tab_node =
session_tracker_.GetTabNodeFromLocalTabId(tab_id, &tab_node_id);
- CHECK_NE(TabNodePool::kInvalidTabNodeID, tab_node_id) << "crbug.com/673618";
+ CHECK_NE(TabNodePool::kInvalidTabNodeID, tab_node_id)
+ << "https://crbug.com/639009";
tab_delegate->SetSyncId(tab_node_id);
+ } else {
+ // Only allowed to allocate sync ids when we have native data, which is only
+ // true when we have a tabbed window. Without a sync id we cannot sync this
+ // data, the tracker cannot even really track it. So don't do any more work.
+ // This effectively breaks syncing custom tabs when the native browser isn't
+ // fully loaded. Ideally this is fixed by saving tab data and sync data
+ // atomically, see https://crbug.com/681921.
+ return;
}
sessions::SessionTab* session_tab =
@@ -604,13 +610,14 @@ void SessionsSyncManager::OnLocalTabModified(SyncedTabDelegate* modified_tab) {
return;
}
+ bool found_tabbed_window = ScanForTabbedWindow();
syncer::SyncChangeList changes;
- AssociateTab(modified_tab, &changes);
+ AssociateTab(modified_tab, found_tabbed_window, &changes);
// Note, we always associate windows because it's possible a tab became
// "interesting" by going to a valid URL, in which case it needs to be added
// to the window's tab information. Similarly, if a tab became
// "uninteresting", we remove it from the window's tab information.
- AssociateWindows(DONT_RELOAD_TABS, &changes);
+ AssociateWindows(DONT_RELOAD_TABS, found_tabbed_window, &changes);
sync_processor_->ProcessSyncChanges(FROM_HERE, changes);
}
@@ -658,7 +665,7 @@ syncer::SyncDataList SessionsSyncManager::GetAllSyncData(
for (auto& tab : win_iter.second->wrapped_window.tabs) {
// TODO(zea): replace with with the correct tab node id once there's a
// sync specific wrapper for SessionTab. This method is only used in
- // tests though, so it's fine for now. crbug.com/662597
+ // tests though, so it's fine for now. https://crbug.com/662597
int tab_node_id = 0;
sync_pb::EntitySpecifics entity;
entity.mutable_session()->CopyFrom(
@@ -801,7 +808,7 @@ bool SessionsSyncManager::InitFromSyncModel(
// to that foreign data (like deletion through garbage collection) to
// trigger a data type error because the tag looking mechanism fails. So
// look for these and delete via remote SyncData, which uses a server id
- // lookup mechanism instead, see crbug.com/604657.
+ // lookup mechanism instead, see https://crbug.com/604657.
bad_foreign_hash_count++;
new_changes->push_back(
syncer::SyncChange(FROM_HERE, SyncChange::ACTION_DELETE, remote));
@@ -1400,4 +1407,25 @@ void SessionsSyncManager::CleanupNavigationTracking() {
}
}
+bool SessionsSyncManager::ScanForTabbedWindow() {
+ for (auto& window_iter_pair :
+ synced_window_delegates_getter()->GetSyncedWindowDelegates()) {
+ if (window_iter_pair.second->IsTypeTabbed()) {
+ const SyncedWindowDelegate* window_delegate = window_iter_pair.second;
+ if (window_delegate->ShouldSync() && window_delegate->GetTabCount() &&
+ window_delegate->HasWindow()) {
+ // When only custom tab windows are open, often we'll have a seemingly
+ // okay type tabbed window, but GetTabAt will return null for each
+ // index. This case is exactly what this method needs to protect
+ // against.
+ for (int j = 0; j < window_delegate->GetTabCount(); ++j) {
+ if (window_delegate->GetTabAt(j))
+ return true;
+ }
+ }
+ }
+ }
+ return false;
+}
+
}; // namespace sync_sessions
diff --git a/chromium/components/sync_sessions/sessions_sync_manager.h b/chromium/components/sync_sessions/sessions_sync_manager.h
index a42a1fd1197..eea4a2f2f84 100644
--- a/chromium/components/sync_sessions/sessions_sync_manager.h
+++ b/chromium/components/sync_sessions/sessions_sync_manager.h
@@ -225,13 +225,16 @@ class SessionsSyncManager : public syncer::SyncableService,
// changes for processing later.
enum ReloadTabsOption { RELOAD_TABS, DONT_RELOAD_TABS };
void AssociateWindows(ReloadTabsOption option,
+ bool has_tabbed_window,
syncer::SyncChangeList* change_output);
// Loads and reassociates the local tabs referenced in |tabs|.
// |change_output| *must* be provided as a link to the SyncChange pipeline
// that exists in the caller's context. This function will append necessary
- // changes for processing later.
+ // changes for processing later. Will only assign a new sync id if there is
+ // a tabbed window, which results in failure for tabs without sync ids yet.
void AssociateTab(SyncedTabDelegate* const tab,
+ bool has_tabbed_window,
syncer::SyncChangeList* change_output);
// Set |session_tab| from |tab_delegate| and |mtime|.
@@ -282,7 +285,7 @@ class SessionsSyncManager : public syncer::SyncableService,
// specifics is the model type, ie us. We need to generate the tag because it
// is not passed over the wire for remote data. The use case this function was
// created for is detecting bad tag hashes from remote data, see
- // crbug.com/604657.
+ // https://crbug.com/604657.
static std::string TagHashFromSpecifics(
const sync_pb::SessionSpecifics& specifics);
@@ -292,6 +295,12 @@ class SessionsSyncManager : public syncer::SyncableService,
void CleanupNavigationTracking();
+ // On Android, it's possible to not have any tabbed windows when only custom
+ // tabs are currently open. This means that there is tab data that will be
+ // restored later, but we cannot access it. This method is an elaborate way to
+ // check if we're currently in that state or not.
+ bool ScanForTabbedWindow();
+
// The client of this sync sessions datatype.
SyncSessionsClient* const sessions_client_;
diff --git a/chromium/components/sync_sessions/sessions_sync_manager_unittest.cc b/chromium/components/sync_sessions/sessions_sync_manager_unittest.cc
index 9e8b709749c..c47117a824d 100644
--- a/chromium/components/sync_sessions/sessions_sync_manager_unittest.cc
+++ b/chromium/components/sync_sessions/sessions_sync_manager_unittest.cc
@@ -61,10 +61,7 @@ const base::Time kTime2 = base::Time::FromInternalValue(120);
const base::Time kTime3 = base::Time::FromInternalValue(130);
const base::Time kTime4 = base::Time::FromInternalValue(140);
const base::Time kTime5 = base::Time::FromInternalValue(150);
-const base::Time kTime6 = base::Time::FromInternalValue(160);
-const base::Time kTime7 = base::Time::FromInternalValue(170);
-const base::Time kTime8 = base::Time::FromInternalValue(180);
-const base::Time kTime9 = base::Time::FromInternalValue(190);
+const base::Time kTime6 = base::Time::FromInternalValue(190);
std::string TabNodeIdToTag(const std::string& machine_tag, int tab_node_id) {
return base::StringPrintf("%s %d", machine_tag.c_str(), tab_node_id);
@@ -874,7 +871,7 @@ TEST_F(SessionsSyncManagerTest, SetSessionTabFromDelegateNavigationIndex) {
tab->set_current_entry_index(kNavs - 2);
sessions::SessionTab session_tab;
- manager()->SetSessionTabFromDelegate(*tab, kTime9, &session_tab);
+ manager()->SetSessionTabFromDelegate(*tab, kTime6, &session_tab);
EXPECT_EQ(6, session_tab.current_navigation_index);
ASSERT_EQ(8u, session_tab.navigations.size());
@@ -894,7 +891,7 @@ TEST_F(SessionsSyncManagerTest, SetSessionTabFromDelegateCurrentInvalid) {
tab->set_current_entry_index(1);
sessions::SessionTab session_tab;
- manager()->SetSessionTabFromDelegate(*tab, kTime9, &session_tab);
+ manager()->SetSessionTabFromDelegate(*tab, kTime6, &session_tab);
EXPECT_EQ(2, session_tab.current_navigation_index);
ASSERT_EQ(3u, session_tab.navigations.size());
@@ -1028,6 +1025,17 @@ TEST_F(SessionsSyncManagerTest, PreserveTabbedDataNoWindows) {
ASSERT_EQ(
restored_tab_id,
out[1].sync_data().GetSpecifics().session().header().window(0).tab(0));
+ out.clear();
+
+ // Now actually resurrect the native data, which will end up having different
+ // native ids, but the tab has the same sync id as before.
+ AddWindow()->OverrideTabAt(0, tab);
+ NavigateTab(tab, kBar1);
+
+ ASSERT_TRUE(ChangeTypeMatches(
+ out, {SyncChange::ACTION_UPDATE, SyncChange::ACTION_UPDATE}));
+ VerifyLocalTabChange(out[0], 3, kBar1);
+ VerifyLocalHeaderChange(out[1], 1, 1);
}
// Ensure that tabbed windows from a previous session are preserved if only
@@ -1058,49 +1066,36 @@ TEST_F(SessionsSyncManagerTest, PreserveTabbedDataCustomTab) {
window->OverrideTabAt(0, custom_tab.get());
InitWithSyncDataTakeOutput(ConvertToRemote(in), &out);
- // The previous session should be preserved, and the transient window should
- // be synced as a new transient window. This means that the original tab
- // node will be updated with its new tab id, a new tab node will be created,
- // and the header will be updated to reflect the two windows and two tabs.
- ASSERT_TRUE(
- ChangeTypeMatches(out, {SyncChange::ACTION_UPDATE, SyncChange::ACTION_ADD,
- SyncChange::ACTION_UPDATE}));
+ // The previous session should be preserved. The transient window cannot be
+ // synced because we do not have enough local data to ensure that we wouldn't
+ // vend the same sync id if our persistent storage didn't match upon the last
+ // shutdown.
+ ASSERT_TRUE(ChangeTypeMatches(
+ out, {SyncChange::ACTION_UPDATE, SyncChange::ACTION_UPDATE}));
VerifyLocalTabChange(out[0], 2, kFoo2);
- VerifyLocalTabChange(out[1], 1, kBar1);
- VerifyLocalHeaderChange(out[2], 2, 2);
-
- // The two windows should have different window types.
- ASSERT_EQ(sync_pb::SessionWindow::TYPE_CUSTOM_TAB, out[2]
- .sync_data()
- .GetSpecifics()
- .session()
- .header()
- .window(0)
- .browser_type());
- ASSERT_EQ(sync_pb::SessionWindow::TYPE_TABBED, out[2]
- .sync_data()
- .GetSpecifics()
- .session()
- .header()
- .window(1)
- .browser_type());
+ VerifyLocalHeaderChange(out[1], 1, 1);
+ out.clear();
- // Verify the tab id of the restored tab is updated and consistent.
- int restored_tab_id =
- out[0].sync_data().GetSpecifics().session().tab().tab_id();
- // SessionId should be rewritten on restore.
- ASSERT_NE(tab->GetSessionId(), restored_tab_id);
- ASSERT_EQ(
- restored_tab_id,
- out[2].sync_data().GetSpecifics().session().header().window(1).tab(0));
+ // Now re-create local data and modify it.
+ TestSyncedWindowDelegate* alive_again = AddWindow();
+ alive_again->OverrideTabAt(0, tab);
+ NavigateTab(tab, kBaz1);
- // Verify the tab id of the custom tab is consistent.
- int custom_tab_id =
- out[1].sync_data().GetSpecifics().session().tab().tab_id();
- ASSERT_EQ(custom_tab->GetSessionId(), custom_tab_id);
- ASSERT_EQ(
- custom_tab_id,
- out[2].sync_data().GetSpecifics().session().header().window(0).tab(0));
+ // The local change should be created and tracked correctly. This doesn't
+ // actually start syncing the custom tab yet, because the tab itself isn't
+ // associated yet.
+ ASSERT_TRUE(ChangeTypeMatches(
+ out, {SyncChange::ACTION_UPDATE, SyncChange::ACTION_UPDATE}));
+ VerifyLocalTabChange(out[0], 3, kBaz1);
+ VerifyLocalHeaderChange(out[1], 1, 1);
+ out.clear();
+
+ // Now trigger OnLocalTabModified() for the custom tab again, it should sync.
+ NavigateTab(custom_tab.get(), kBar2);
+ ASSERT_TRUE(ChangeTypeMatches(
+ out, {SyncChange::ACTION_ADD, SyncChange::ACTION_UPDATE}));
+ VerifyLocalTabChange(out[0], 2, kBar2);
+ VerifyLocalHeaderChange(out[1], 2, 2);
}
// Tests MergeDataAndStartSyncing with sync data but no local data.
@@ -1790,7 +1785,7 @@ TEST_F(SessionsSyncManagerTest, MergeDeletesTabMissingTabId) {
// Verifies that we drop both headers and tabs during merge if their stored tag
// hash doesn't match a computer tag hash. This mitigates potential failures
-// while cleaning up bad foreign data, see crbug.com/604657.
+// while cleaning up bad foreign data, see https://crbug.com/604657.
TEST_F(SessionsSyncManagerTest, MergeDeletesBadHash) {
SyncDataList foreign_data;
std::vector<SessionID::id_type> empty_ids;
diff --git a/chromium/components/sync_sessions/synced_session_tracker.cc b/chromium/components/sync_sessions/synced_session_tracker.cc
index 4b2975ab234..2a6960c7500 100644
--- a/chromium/components/sync_sessions/synced_session_tracker.cc
+++ b/chromium/components/sync_sessions/synced_session_tracker.cc
@@ -296,7 +296,7 @@ void SyncedSessionTracker::PutTabInWindow(const std::string& session_tag,
// TODO(zea): remove this once PutTabInWindow isn't crashing anymore.
CHECK(tab) << " Unable to find tab " << tab_id
<< " within unmapped tabs or previously mapped windows."
- << " crbug.com/639009";
+ << " https://crbug.com/639009";
}
tab->window_id.set_id(window_id);
@@ -316,7 +316,8 @@ void SyncedSessionTracker::OnTabNodeSeen(const std::string& session_tag,
sessions::SessionTab* SyncedSessionTracker::GetTab(
const std::string& session_tag,
SessionID::id_type tab_id) {
- CHECK_NE(TabNodePool::kInvalidTabNodeID, tab_id) << "crbug.com/673618";
+ CHECK_NE(TabNodePool::kInvalidTabNodeID, tab_id)
+ << "https://crbug.com/639009";
sessions::SessionTab* tab_ptr = nullptr;
auto iter = synced_tab_map_[session_tag].find(tab_id);
if (iter != synced_tab_map_[session_tag].end()) {
diff --git a/chromium/components/sync_sessions/synced_session_tracker.h b/chromium/components/sync_sessions/synced_session_tracker.h
index 84269352e2d..a8c9480f48f 100644
--- a/chromium/components/sync_sessions/synced_session_tracker.h
+++ b/chromium/components/sync_sessions/synced_session_tracker.h
@@ -131,7 +131,7 @@ class SyncedSessionTracker {
// |tab_id| for the session specified with |session_tag|.
// Note: Ownership of the SessionTab remains within the SyncedSessionTracker.
// TODO(zea): Replace SessionTab with a Sync specific wrapper.
- // crbug.com/662597
+ // https://crbug.com/662597
sessions::SessionTab* GetTab(const std::string& session_tag,
SessionID::id_type tab_id);
diff --git a/chromium/components/sync_sessions/tab_node_pool.cc b/chromium/components/sync_sessions/tab_node_pool.cc
index 1b4c1f88f62..19bdd438b1c 100644
--- a/chromium/components/sync_sessions/tab_node_pool.cc
+++ b/chromium/components/sync_sessions/tab_node_pool.cc
@@ -124,7 +124,7 @@ void TabNodePool::CleanupTabNodes(std::set<int>* deleted_node_ids) {
// If number of free nodes exceed kFreeNodesHighWatermark,
// delete sync nodes till number reaches kFreeNodesLowWatermark.
// Note: This logic is to mitigate temporary disassociation issues with old
- // clients: http://crbug.com/259918. Newer versions do not need this.
+ // clients: https://crbug.com/259918. Newer versions do not need this.
if (free_nodes_pool_.size() > kFreeNodesHighWatermark) {
for (std::set<int>::iterator free_it = free_nodes_pool_.begin();
free_it != free_nodes_pool_.end();) {