Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(24)

Issue 6901031: Profile shouldn't own Session/TabRestore services. (Closed)

Created:
9 years, 8 months ago by Torne
Modified:
9 years, 7 months ago
CC:
chromium-reviews, ncarter (slow), jam, Raghu Simha, idana, achuith+watch_chromium.org, brettw-cc_chromium.org, kkania, Avi (use Gerrit), rginda+watch_chromium.org, pam+watch_chromium.org, Paweł Hajdan Jr., tim (not reviewing)
Visibility:
Public.

Description

Profile shouldn't own Session/TabRestore services. BaseSessionService is now a ProfileKeyedService. Its subclasses, SessionService and TabRestoreService, are now accessed through factories. BUG=77155 TEST=existing tests Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=83325

Patch Set 1 #

Patch Set 2 : Fix mac build #

Total comments: 2

Patch Set 3 : Lint #

Patch Set 4 : Rebase after gtk history menu introduction #

Patch Set 5 : Rebase again and hope upload works this time #

Unified diffs Side-by-side diffs Delta from patch set Stats (+369 lines, -235 lines) Patch
M chrome/browser/app_controller_mac.mm View 1 2 3 4 chunks +6 lines, -3 lines 0 comments Download
M chrome/browser/automation/automation_provider_observers.cc View 1 2 3 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/automation/testing_automation_provider.cc View 1 2 3 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/browsing_data_remover.cc View 1 2 3 2 chunks +6 lines, -2 lines 0 comments Download
M chrome/browser/jumplist_win.cc View 4 chunks +11 lines, -4 lines 0 comments Download
M chrome/browser/profiles/profile.h View 1 2 3 3 chunks +0 lines, -25 lines 0 comments Download
M chrome/browser/profiles/profile.cc View 1 2 3 2 chunks +0 lines, -21 lines 0 comments Download
M chrome/browser/profiles/profile_impl.h View 1 2 3 5 chunks +1 line, -15 lines 0 comments Download
M chrome/browser/profiles/profile_impl.cc View 1 2 3 7 chunks +7 lines, -40 lines 0 comments Download
M chrome/browser/profiles/profile_manager.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/profiles/profile_manager.cc View 1 2 3 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/sessions/base_session_service.h View 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/sessions/session_restore.cc View 4 chunks +6 lines, -3 lines 0 comments Download
M chrome/browser/sessions/session_restore_browsertest.cc View 3 chunks +5 lines, -2 lines 0 comments Download
A chrome/browser/sessions/session_service_factory.h View 1 chunk +60 lines, -0 lines 0 comments Download
A chrome/browser/sessions/session_service_factory.cc View 1 2 1 chunk +56 lines, -0 lines 0 comments Download
M chrome/browser/sessions/tab_restore_service.cc View 2 chunks +5 lines, -3 lines 0 comments Download
M chrome/browser/sessions/tab_restore_service_browsertest.cc View 9 chunks +16 lines, -8 lines 0 comments Download
A chrome/browser/sessions/tab_restore_service_factory.h View 1 chunk +35 lines, -0 lines 0 comments Download
A chrome/browser/sessions/tab_restore_service_factory.cc View 1 2 1 chunk +40 lines, -0 lines 0 comments Download
M chrome/browser/sync/glue/session_model_associator.cc View 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/sync/profile_sync_service_session_unittest.cc View 3 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/ui/browser.cc View 1 2 3 17 chunks +51 lines, -50 lines 0 comments Download
M chrome/browser/ui/browser_init.cc View 2 chunks +10 lines, -7 lines 0 comments Download
M chrome/browser/ui/cocoa/history_menu_bridge.mm View 1 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/history_menu_cocoa_controller.mm View 2 chunks +4 lines, -1 line 0 comments Download
M chrome/browser/ui/gtk/global_history_menu.cc View 1 2 3 3 chunks +5 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/frame/browser_view.cc View 1 2 3 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/new_tab_ui.cc View 1 2 3 2 chunks +5 lines, -3 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/test/live_sync/live_sessions_sync_test.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/test/testing_profile.h View 1 2 3 4 chunks +0 lines, -10 lines 0 comments Download
M chrome/test/testing_profile.cc View 1 2 3 5 chunks +4 lines, -17 lines 0 comments Download
M content/browser/tab_contents/navigation_controller_unittest.cc View 4 chunks +7 lines, -7 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Torne
Another one of these :) Scott, can you check I haven't done anything really stupid ...
9 years, 8 months ago (2011-04-27 11:30:28 UTC) #1
Avi (use Gerrit)
content LGTM
9 years, 8 months ago (2011-04-27 12:10:49 UTC) #2
Miranda Callahan
On 2011/04/27 12:10:49, Avi wrote: > content LGTM Profiles LGTM -- I love these CL's ...
9 years, 8 months ago (2011-04-27 12:46:21 UTC) #3
sky
Session changes LGTM http://codereview.chromium.org/6901031/diff/2001/chrome/browser/sessions/session_service_factory.cc File chrome/browser/sessions/session_service_factory.cc (right): http://codereview.chromium.org/6901031/diff/2001/chrome/browser/sessions/session_service_factory.cc#newcode55 chrome/browser/sessions/session_service_factory.cc:55: nit: nuke this line. http://codereview.chromium.org/6901031/diff/2001/chrome/browser/sessions/tab_restore_service_factory.cc File ...
9 years, 8 months ago (2011-04-27 15:58:27 UTC) #4
commit-bot: I haz the power
Try job failure for 6901031-4004 on linux: http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux&number=23777
9 years, 8 months ago (2011-04-27 16:41:49 UTC) #5
commit-bot: I haz the power
Can't process patch for file chrome/browser/profiles/profile_manager.h. File's status is None, patchset upload is incomplete.
9 years, 8 months ago (2011-04-28 10:46:44 UTC) #6
commit-bot: I haz the power
9 years, 8 months ago (2011-04-28 11:50:18 UTC) #7
Change committed as 83325

Powered by Google App Engine
This is Rietveld 408576698