diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2018-01-31 16:33:43 +0100 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2018-02-06 16:33:22 +0000 |
commit | da51f56cc21233c2d30f0fe0d171727c3102b2e0 (patch) | |
tree | 4e579ab70ce4b19bee7984237f3ce05a96d59d83 /chromium/components/sync_sessions | |
parent | c8c2d1901aec01e934adf561a9fdf0cc776cdef8 (diff) | |
download | qtwebengine-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')
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();) { |