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

Issue 1342743002: Remove Profile and HostDesktopType dependencies from core TabRestore code (Closed)

Created:
5 years, 3 months ago by blundell
Modified:
5 years, 3 months ago
Reviewers:
sky
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove Profile and HostDesktopType dependencies from core TabRestore code The remaining //chrome-level dependencies in core TabRestore code are due to static methods of TabRestoreServiceDelegate that take in a Profile* and a chrome::HostDesktopType. This CL replaces the usage of these methods in core TabRestore code with usage of new methods that have no //chrome-level dependencies: (1) Creates corresponding virtual methods on TabRestoreServiceClient, removing the need to pass a Profile* (since TabRestoreServiceClient is itself an abstraction of Profile). (2) Changes the reference to chrome::HostDesktopType to be an opaque integer representing the host desktop type. This opaque integer is not used by core tab restore code. To move TabRestoreServiceDelegate toward componentization, this CL also removes those static methods entirely. Once the usage of the TabRestoreServiceDelegate static methods is removed from core TabRestore code, the only remaining usage of these methods is in desktop-specific code that knows about Browser. Thus, we move the declaration of the static methods into BrowserTabRestoreServiceDelegate. BUG=530162, 530163 Committed: https://crrev.com/7bf49a3df73ba892210a387735b6d6b52dd88600 Cr-Commit-Position: refs/heads/master@{#348663}

Patch Set 1 #

Patch Set 2 : Self-review #

Patch Set 3 : Self-review, round 2 #

Patch Set 4 : Fix Mac #

Unified diffs Side-by-side diffs Delta from patch set Stats (+165 lines, -130 lines) Patch
M chrome/browser/extensions/api/sessions/sessions_api.cc View 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/sessions/chrome_tab_restore_service_client.h View 1 1 chunk +8 lines, -0 lines 0 comments Download
M chrome/browser/sessions/chrome_tab_restore_service_client.cc View 1 2 chunks +45 lines, -0 lines 0 comments Download
M chrome/browser/sessions/in_memory_tab_restore_service.h View 2 chunks +2 lines, -3 lines 0 comments Download
M chrome/browser/sessions/in_memory_tab_restore_service.cc View 3 chunks +3 lines, -4 lines 0 comments Download
M chrome/browser/sessions/persistent_tab_restore_service.h View 2 chunks +2 lines, -5 lines 0 comments Download
M chrome/browser/sessions/persistent_tab_restore_service.cc View 3 chunks +7 lines, -8 lines 0 comments Download
M chrome/browser/sessions/persistent_tab_restore_service_unittest.cc View 2 chunks +0 lines, -2 lines 0 comments Download
M chrome/browser/sessions/tab_restore_service.h View 1 3 chunks +10 lines, -9 lines 0 comments Download
M chrome/browser/sessions/tab_restore_service_delegate.h View 2 chunks +0 lines, -21 lines 0 comments Download
M chrome/browser/sessions/tab_restore_service_factory.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/sessions/tab_restore_service_helper.h View 1 2 5 chunks +12 lines, -17 lines 0 comments Download
M chrome/browser/sessions/tab_restore_service_helper.cc View 8 chunks +10 lines, -12 lines 0 comments Download
D chrome/browser/ui/android/tab_restore_service_delegate_android.cc View 1 chunk +0 lines, -31 lines 0 comments Download
M chrome/browser/ui/browser_tab_restore_service_delegate.h View 2 chunks +19 lines, -0 lines 0 comments Download
M chrome/browser/ui/browser_tab_restore_service_delegate.cc View 1 2 chunks +4 lines, -6 lines 0 comments Download
M chrome/browser/ui/browser_tabrestore_browsertest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/cocoa/history_menu_bridge_unittest.mm View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/toolbar/recent_tabs_sub_menu_model_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/chrome_browser_ui.gypi View 1 chunk +0 lines, -1 line 0 comments Download
M components/sessions/core/tab_restore_service_client.h View 1 2 chunks +34 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 16 (6 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1342743002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1342743002/40001
5 years, 3 months ago (2015-09-14 11:17:12 UTC) #2
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/112375)
5 years, 3 months ago (2015-09-14 11:44:57 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1342743002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1342743002/60001
5 years, 3 months ago (2015-09-14 11:51:40 UTC) #6
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 3 months ago (2015-09-14 12:41:24 UTC) #8
blundell
5 years, 3 months ago (2015-09-14 13:29:08 UTC) #10
sky
LGTM
5 years, 3 months ago (2015-09-14 16:27:49 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1342743002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1342743002/60001
5 years, 3 months ago (2015-09-14 18:16:46 UTC) #13
commit-bot: I haz the power
Committed patchset #4 (id:60001)
5 years, 3 months ago (2015-09-14 18:38:22 UTC) #14
commit-bot: I haz the power
Patchset 4 (id:??) landed as https://crrev.com/7bf49a3df73ba892210a387735b6d6b52dd88600 Cr-Commit-Position: refs/heads/master@{#348663}
5 years, 3 months ago (2015-09-14 18:39:02 UTC) #15
commit-bot: I haz the power
5 years, 3 months ago (2015-09-23 12:34:45 UTC) #16
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/7bf49a3df73ba892210a387735b6d6b52dd88600
Cr-Commit-Position: refs/heads/master@{#348663}

Powered by Google App Engine
This is Rietveld 408576698