chrome/browser/sessions/session_restore_observer.h - Issue 2935183002: [TabMetrics] Add signals that mark the start and end of session restore.

Side by Side Diff: chrome/browser/sessions/session_restore_observer.h

Issue 2935183002: [TabMetrics] Add signals that mark the start and end of session restore. (Closed)

Patch Set: Address lpy@'s comments Created 3 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« no previous file with comments | « chrome/browser/resource_coordinator/tab_manager_unittest.cc ('k') | chrome/browser/sessions/session_restore_observer.cc » ('j') | chrome/browser/sessions/session_restore_observer.cc » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 // Copyright 2017 The Chromium Authors. All rights reserved.

	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 #ifndef CHROME_BROWSER_SESSIONS_SESSION_RESTORE_OBSERVER_H_

	6 #define CHROME_BROWSER_SESSIONS_SESSION_RESTORE_OBSERVER_H_

	7

	8 #include <unordered_set>

	9

	10 #include "base/observer_list.h"

	11

	12 // Observer of events during session restore.

	13 class SessionRestoreObserver {

	14 public:

	15 virtual void OnTabLoadingStarted() {}
	sky 2017/06/26 15:34:11 Clearly document at what point these functions are Clearly document at what point these functions are called. sky 2017/06/26 15:34:11 Given these functions are on the SessionRestore in Given these functions are on the SessionRestore interface I think it better to name them something like OnSessionRestoreStartedLoadingTabs and OnSessionRestoreFinishedLoadingTabs. sky 2017/06/26 15:34:11 Please add test coverage that these functions are Please add test coverage that these functions are called. ducbui 2017/06/26 19:59:26 Done. Show quoted text On 2017/06/26 15:34:11, sky wrote: > Please add test coverage that these functions are called. Done. ducbui 2017/06/26 19:59:26 Done. Show quoted text On 2017/06/26 15:34:11, sky wrote: > Given these functions are on the SessionRestore interface I think it better to > name them something like OnSessionRestoreStartedLoadingTabs and > OnSessionRestoreFinishedLoadingTabs. Done. ducbui 2017/06/26 19:59:26 Done. Show quoted text On 2017/06/26 15:34:11, sky wrote: > Clearly document at what point these functions are called. Done.
	16 virtual void OnTabLoadingEnded() {}

	17

	18 static base::ObserverList<SessionRestoreObserver>& observers();

	19

	20 static void AddObserver(SessionRestoreObserver* observer);
	sky 2017/06/26 15:34:11 Move these functions onto SessionRestore. Also, t Move these functions onto SessionRestore. Also, the callback in SessionRestore should be replaced by your observer, but that can be done separately. ducbui 2017/06/26 19:59:26 Done. Show quoted text On 2017/06/26 15:34:11, sky wrote: > Move these functions onto SessionRestore. Done. Show quoted text > Also, the callback in SessionRestore should be replaced by your observer, but > that can be done separately. Do you mean the callback in SessionRestore::RegisterOnSessionRestoredCallback()? Do you want me to include the replacement in this CL? sky 2017/06/26 21:38:08 Exactly. Show quoted text On 2017/06/26 19:59:26, ducbui wrote: > On 2017/06/26 15:34:11, sky wrote: > > Move these functions onto SessionRestore. > > Done. > > > Also, the callback in SessionRestore should be replaced by your observer, but > > that can be done separately. > > Do you mean the callback in SessionRestore::RegisterOnSessionRestoredCallback()? Exactly. Show quoted text > Do you want me to include the replacement in this CL? That would be great, but I'm ok if you want to push that to a follow on cl.
	21 static void RemoveObserver(SessionRestoreObserver* observer);

	22

	23 private:

	24 static base::ObserverList<SessionRestoreObserver>* observers_;

	25 };

	26

	27 #endif // CHROME_BROWSER_SESSIONS_SESSION_RESTORE_OBSERVER_H_

OLD	NEW