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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 14497003: Moves TabNavigation into components/sessions and renames (Closed)

Created:
7 years, 8 months ago by sky
Modified:
7 years, 7 months ago
CC:
chromium-reviews, dbeam+watch-ntp_chromium.org, melevin, akalin, Raghu Simha, marja+watch_chromium.org, dhollowa+watch_chromium.org, dougw+watch_chromium.org, gideonwald, sail+watch_chromium.org, sreeram, dominich, Aaron Boodman, David Black, tim (not reviewing), samarth+watch_chromium.org, kmadhusu+watch_chromium.org, chromium-apps-reviews_chromium.org, haitaol1, estade+watch_chromium.org, Jered, pedrosimonetti+watch_chromium.org
Visibility:
Public.

Description

Moves TabNavigation into components/sessions and renames SerializedNaviationEntry. FreezeDriedNavigationEntry is probably an even better name, but I couldn't bring myself to such a long and wordy class name. Additionally did the following changes to make it easier to move: . exposed a bunch of getters on SerializedNavigationEntry so that you don't need to go through SerializedNavigationTestHelper. . Make WriteToPickle take the max size. This way the code doesn't have a dependency on SessionID. BUG=none TEST=none Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=197086

Patch Set 1 #

Patch Set 2 : get component_unittests working #

Patch Set 3 : sync_integration_tests #

Patch Set 4 : Remove unnecessary const #

Patch Set 5 : Fix android #

Patch Set 6 : Attempt at fixing win/max #

Patch Set 7 : Move protobuf dependency from test to sessions #

Patch Set 8 : Add protobuf back to tests #

Patch Set 9 : Remove unneeded declarations/includes #

Patch Set 10 : OWNERs tweaks and 30% similarity #

Patch Set 11 : similarity 20% #

Patch Set 12 : Cleanup #

Total comments: 8

Patch Set 13 : components->sessions #

Patch Set 14 : Reorder using #

Patch Set 15 : webkit_support only on non-ios #

Patch Set 16 : Really remove webkit_support #

Unified diffs Side-by-side diffs Delta from patch set Stats (+636 lines, -1721 lines) Patch
M chrome/DEPS View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/extensions/api/session_restore/session_restore_api.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/jumplist_win.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/search/search.h View 1 chunk +0 lines, -5 lines 0 comments Download
M chrome/browser/search/search.cc View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +2 lines, -3 lines 0 comments Download
M chrome/browser/sessions/base_session_service.h View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +12 lines, -7 lines 0 comments Download
M chrome/browser/sessions/base_session_service.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +7 lines, -3 lines 0 comments Download
M chrome/browser/sessions/persistent_tab_restore_service.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/sessions/persistent_tab_restore_service_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 10 chunks +17 lines, -16 lines 0 comments Download
M chrome/browser/sessions/session_command.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/sessions/session_restore_android.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sessions/session_restore_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 7 chunks +16 lines, -15 lines 0 comments Download
M chrome/browser/sessions/session_service.h View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +11 lines, -9 lines 0 comments Download
M chrome/browser/sessions/session_service.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 10 chunks +17 lines, -15 lines 0 comments Download
M chrome/browser/sessions/session_service_test_helper.h View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +7 lines, -3 lines 0 comments Download
M chrome/browser/sessions/session_service_test_helper.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +7 lines, -4 lines 0 comments Download
M chrome/browser/sessions/session_service_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 29 chunks +114 lines, -87 lines 0 comments Download
M chrome/browser/sessions/session_types.h View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +8 lines, -103 lines 0 comments Download
M chrome/browser/sessions/session_types.cc View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +4 lines, -456 lines 0 comments Download
D chrome/browser/sessions/session_types_test_helper.h View 1 chunk +0 lines, -65 lines 0 comments Download
D chrome/browser/sessions/session_types_test_helper.cc View 1 chunk +0 lines, -103 lines 0 comments Download
M chrome/browser/sessions/session_types_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +5 lines, -217 lines 0 comments Download
M chrome/browser/sessions/tab_restore_service.h View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/sessions/tab_restore_service_delegate.h View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +6 lines, -3 lines 0 comments Download
M chrome/browser/sessions/tab_restore_service_helper.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync/glue/session_model_associator.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/sync/glue/session_model_associator_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 7 chunks +17 lines, -16 lines 0 comments Download
M chrome/browser/sync/glue/synced_session_tracker_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/sync/profile_sync_service_session_unittest.cc View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -4 lines 0 comments Download
M chrome/browser/sync/test/integration/sessions_helper.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/sync/test/integration/sessions_helper.cc View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +10 lines, -14 lines 0 comments Download
M chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +3 lines, -4 lines 0 comments Download
M chrome/browser/ui/browser.h View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/browser_tab_restore_service_delegate.h View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/browser_tab_restore_service_delegate.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/browser_tabrestore.h View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +8 lines, -4 lines 0 comments Download
M chrome/browser/ui/browser_tabrestore.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 5 chunks +5 lines, -4 lines 0 comments Download
M chrome/browser/ui/cocoa/history_menu_bridge.mm View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/cocoa/history_menu_bridge_unittest.mm View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/gtk/global_history_menu.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/search/instant_controller.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc View 1 2 3 4 5 6 7 8 9 10 11 12 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/browser/ui/webui/ntp/foreign_session_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/webui/ntp/recently_closed_tabs_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome.gyp View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 2 chunks +1 line, -2 lines 0 comments Download
M components/OWNERS View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -0 lines 0 comments Download
M components/components.gyp View 1 chunk +1 line, -0 lines 0 comments Download
M components/components_tests.gypi View 1 7 2 chunks +9 lines, -2 lines 0 comments Download
A + components/sessions.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +32 lines, -10 lines 0 comments Download
A + components/sessions/DEPS View 1 2 3 4 5 6 7 8 9 10 1 chunk +5 lines, -4 lines 0 comments Download
A + components/sessions/serialized_navigation_entry.h View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +54 lines, -171 lines 0 comments Download
A + components/sessions/serialized_navigation_entry.cc View 1 2 3 4 5 6 7 8 9 10 11 12 11 chunks +31 lines, -99 lines 0 comments Download
A components/sessions/serialized_navigation_entry_test_helper.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +62 lines, -0 lines 0 comments Download
A + components/sessions/serialized_navigation_entry_test_helper.cc View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +36 lines, -56 lines 0 comments Download
A + components/sessions/serialized_navigation_entry_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 8 chunks +76 lines, -179 lines 0 comments Download
A + components/sessions/sessions_export.h View 1 2 3 4 5 6 7 8 9 1 chunk +10 lines, -10 lines 0 comments Download

Messages

Total messages: 15 (0 generated)
sky
joi: move to components atwilson: sync related changes sreeram: I'm promoting to search::kInstantExtendedSearchTermsKey to components::kSearchTermsKey. ...
7 years, 8 months ago (2013-04-26 16:54:40 UTC) #1
Andrew T Wilson (Slow)
Deferring to Nicolas for reviewing the browser/sync stuff since he owns that code.
7 years, 8 months ago (2013-04-26 17:54:28 UTC) #2
sreeram
instant/search_terms rubberstamp lgtm
7 years, 8 months ago (2013-04-26 17:59:16 UTC) #3
Jói
I looked only at //components. https://codereview.chromium.org/14497003/diff/58005/components/sessions/serialized_navigation_entry.cc File components/sessions/serialized_navigation_entry.cc (right): https://codereview.chromium.org/14497003/diff/58005/components/sessions/serialized_navigation_entry.cc#newcode1 components/sessions/serialized_navigation_entry.cc:1: // Copyright 2012 The ...
7 years, 8 months ago (2013-04-26 21:55:47 UTC) #4
Jói
Since those are all nits (apart from the namespace thing, which is better done in ...
7 years, 8 months ago (2013-04-26 21:59:23 UTC) #5
Andrew T Wilson (Slow)
lgtm
7 years, 8 months ago (2013-04-27 14:57:29 UTC) #6
sky
Moved to sessions namespace and incorporated other feedback.
7 years, 7 months ago (2013-04-29 15:12:02 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sky@chromium.org/14497003/19012
7 years, 7 months ago (2013-04-29 15:12:23 UTC) #8
commit-bot: I haz the power
Retried try job too often on ios_rel_device for step(s) compile http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_rel_device&number=44911
7 years, 7 months ago (2013-04-29 15:34:27 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sky@chromium.org/14497003/59125
7 years, 7 months ago (2013-04-29 16:09:51 UTC) #10
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 7 months ago (2013-04-29 16:25:27 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sky@chromium.org/14497003/59125
7 years, 7 months ago (2013-04-29 16:43:38 UTC) #12
commit-bot: I haz the power
Retried try job too often on ios_dbg_simulator for step(s) base_unittests, compile, content_unittests, crypto_unittests, googleurl_unittests, media_unittests, ...
7 years, 7 months ago (2013-04-29 17:12:32 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sky@chromium.org/14497003/59125
7 years, 7 months ago (2013-04-29 17:13:46 UTC) #14
commit-bot: I haz the power
7 years, 7 months ago (2013-04-29 18:13:04 UTC) #15
Message was sent while issue was closed. 
Change committed as 197086

Powered by Google App Engine
This is Rietveld 408576698