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

Issue 815983002: Topsites become keyedService based. (Closed)

Created:
6 years ago by Jitu( very slow this week)
Modified:
5 years, 11 months ago
CC:
aandrey+blink_chromium.org, browser-components-watch_chromium.org, chrome-apps-syd-reviews_chromium.org, chromium-apps-reviews_chromium.org, chromium-reviews, dbeam+watch-ntp_chromium.org, David Black, devtools-reviews_chromium.org, dhollowa+watch_chromium.org, donnd+watch_chromium.org, dougw+watch_chromium.org, estade+watch_chromium.org, extensions-reviews_chromium.org, Jered, jfweitz+watch_chromium.org, kmadhusu+watch_chromium.org, melevin+watch_chromium.org, nshaik, paulirish+reviews_chromium.org, pedrosimonetti+watch_chromium.org, pfeldman, samarth+watch_chromium.org, skanuj+watch_chromium.org, James Su, tfarina, vsevik, yurys
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Topsites become keyedService based. TopSites was created and owned by profile. This patch creates a TopSitesServicefactory which is keyedService. So now instead of calling GetTopSites() from profile, can get the TopSites from TopSitesFactory. BUG=435501 Committed: https://crrev.com/30f03390995328247e3642aa5cd2cc9aae781b45 Cr-Commit-Position: refs/heads/master@{#313468}

Patch Set 1 #

Patch Set 2 : fix #

Total comments: 56

Patch Set 3 : Fix comments #

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : Fix the Review comments #

Patch Set 7 : Fix #

Patch Set 8 : fix #

Total comments: 29

Patch Set 9 : Fix the review comments #

Total comments: 9

Patch Set 10 : Fix review comments #

Total comments: 66

Patch Set 11 : Fixed the Review comments #

Total comments: 4

Patch Set 12 : Fix comments #

Total comments: 7

Patch Set 13 : For trybot #

Total comments: 23

Patch Set 14 : Rebased and fixed unit test fail #

Total comments: 27

Patch Set 15 : Fixed review comments #

Total comments: 4

Patch Set 16 : Fixed review comments #

Total comments: 2

Patch Set 17 : fixed comments #

Patch Set 18 : Rebased #

Total comments: 3

Patch Set 19 : Remove extra inclusion from testing_profile.h #

Unified diffs Side-by-side diffs Delta from patch set Stats (+311 lines, -185 lines) Patch
M chrome/browser/android/dev_tools_manager_delegate_android.cc View 1 2 3 4 5 6 7 8 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/android/dev_tools_server.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/android/most_visited_sites.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 6 chunks +8 lines, -5 lines 0 comments Download
M chrome/browser/android/provider/chrome_browser_provider.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/android/provider/chrome_browser_provider.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/autocomplete/zero_suggest_provider.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/autocomplete/zero_suggest_provider_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 8 chunks +21 lines, -9 lines 0 comments Download
M chrome/browser/devtools/browser_list_tabcontents_provider.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/devtools/chrome_devtools_manager_delegate.cc View 1 2 3 4 5 6 7 8 9 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/extensions/api/top_sites/top_sites_api.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/extensions/api/top_sites/top_sites_apitest.cc View 1 2 3 4 5 6 7 8 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/history/expire_history_backend_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 5 chunks +9 lines, -7 lines 0 comments Download
M chrome/browser/history/history_backend.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/history/history_service.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/history/top_sites.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 3 chunks +3 lines, -10 lines 0 comments Download
M chrome/browser/history/top_sites.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +10 lines, -14 lines 0 comments Download
A chrome/browser/history/top_sites_factory.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +49 lines, -0 lines 0 comments Download
A chrome/browser/history/top_sites_factory.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +51 lines, -0 lines 0 comments Download
M chrome/browser/history/top_sites_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/history/top_sites_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +4 lines, -2 lines 0 comments Download
M chrome/browser/history/top_sites_impl_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/jumplist_win.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +5 lines, -2 lines 0 comments Download
M chrome/browser/profiles/off_the_record_profile_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/profiles/off_the_record_profile_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +0 lines, -8 lines 0 comments Download
M chrome/browser/profiles/profile.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +0 lines, -11 lines 0 comments Download
M chrome/browser/profiles/profile_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +0 lines, -4 lines 0 comments Download
M chrome/browser/profiles/profile_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 3 chunks +0 lines, -16 lines 0 comments Download
M chrome/browser/search/instant_service.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 6 chunks +11 lines, -5 lines 0 comments Download
M chrome/browser/search/instant_service_factory.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/browser/thumbnails/thumbnail_list_source.cc View 1 2 3 4 5 6 7 8 9 2 chunks +7 lines, -4 lines 0 comments Download
M chrome/browser/thumbnails/thumbnail_service_factory.cc View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/browser/thumbnails/thumbnail_service_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 7 chunks +7 lines, -6 lines 0 comments Download
M chrome/browser/thumbnails/thumbnail_service_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +21 lines, -9 lines 0 comments Download
M chrome/browser/ui/app_list/test/fake_profile.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/app_list/test/fake_profile.cc View 1 chunk +0 lines, -8 lines 0 comments Download
M chrome/browser/ui/browser.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/ui/views/frame/global_menu_bar_x11.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/frame/global_menu_bar_x11.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/webui/ntp/most_visited_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 8 chunks +14 lines, -7 lines 0 comments Download
M chrome/browser/ui/webui/ntp/suggestions_page_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +4 lines, -2 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/test/base/testing_profile.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 3 chunks +0 lines, -9 lines 0 comments Download
M chrome/test/base/testing_profile.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 8 chunks +45 lines, -26 lines 0 comments Download

Messages

Total messages: 80 (24 generated)
Jitu( very slow this week)
PTAL Is this you want to be. Please let me know any suggestions. Thanks
6 years ago (2014-12-19 12:58:06 UTC) #2
sdefresne
I've made lots of comments, but I still think this is going into the right ...
6 years ago (2014-12-19 15:11:42 UTC) #3
Jitu( very slow this week)
Thanks ... I am taking care all your comments Just some doubt chrome/browser/history/top_sites_service_factory.h:15: class TopSitesServiceFactory ...
6 years ago (2014-12-23 10:44:49 UTC) #6
sdefresne
On 2014/12/23 at 10:44:49, jitendra.ks wrote: > Thanks ... > I am taking care all ...
6 years ago (2014-12-23 10:48:02 UTC) #7
Jitu( very slow this week)
PTAL https://codereview.chromium.org/815983002/diff/20001/chrome/browser/history/chrome_history_client_factory.cc File chrome/browser/history/chrome_history_client_factory.cc (right): https://codereview.chromium.org/815983002/diff/20001/chrome/browser/history/chrome_history_client_factory.cc#newcode38 chrome/browser/history/chrome_history_client_factory.cc:38: DependsOn(BookmarkModelFactory::GetInstance()); On 2014/12/19 15:11:40, sdefresne wrote: > You ...
5 years, 12 months ago (2014-12-26 14:04:29 UTC) #20
sdefresne
https://codereview.chromium.org/815983002/diff/400001/chrome/browser/android/dev_tools_manager_delegate_android.cc File chrome/browser/android/dev_tools_manager_delegate_android.cc (right): https://codereview.chromium.org/815983002/diff/400001/chrome/browser/android/dev_tools_manager_delegate_android.cc#newcode343 chrome/browser/android/dev_tools_manager_delegate_android.cc:343: if (top_sites.get()) { nit: "if (top_sites)" should work, here ...
5 years, 12 months ago (2014-12-29 09:48:14 UTC) #21
Jitu( very slow this week)
Fixed all the review comments... PTAL https://codereview.chromium.org/815983002/diff/400001/chrome/browser/android/dev_tools_manager_delegate_android.cc File chrome/browser/android/dev_tools_manager_delegate_android.cc (right): https://codereview.chromium.org/815983002/diff/400001/chrome/browser/android/dev_tools_manager_delegate_android.cc#newcode343 chrome/browser/android/dev_tools_manager_delegate_android.cc:343: if (top_sites.get()) { ...
5 years, 11 months ago (2014-12-30 10:09:03 UTC) #25
Jitu( very slow this week)
bauerb@chromium.org: Please review changes in cpu@chromium.org: Please review changes in
5 years, 11 months ago (2015-01-06 10:49:02 UTC) #27
Bernhard Bauer
The CL description seems to be outdated. Also, FTR: It's customary to wait for an ...
5 years, 11 months ago (2015-01-06 11:10:31 UTC) #28
Bernhard Bauer
Also, Rietveld is a bit overzealous in parsing my comment. Not LGTM in this state.
5 years, 11 months ago (2015-01-06 11:11:13 UTC) #29
Jitu( very slow this week)
On 2015/01/06 11:10:31, Bernhard Bauer wrote: > The CL description seems to be outdated. > ...
5 years, 11 months ago (2015-01-07 05:30:31 UTC) #30
Jitu( very slow this week)
PTAL https://codereview.chromium.org/815983002/diff/480001/chrome/browser/history/top_sites_factory.cc File chrome/browser/history/top_sites_factory.cc (right): https://codereview.chromium.org/815983002/diff/480001/chrome/browser/history/top_sites_factory.cc#newcode42 chrome/browser/history/top_sites_factory.cc:42: scoped_refptr<RefcountedKeyedService> On 2015/01/06 11:10:30, Bernhard Bauer wrote: > ...
5 years, 11 months ago (2015-01-08 09:38:25 UTC) #31
Bernhard Bauer
https://codereview.chromium.org/815983002/diff/400001/chrome/browser/android/dev_tools_manager_delegate_android.cc File chrome/browser/android/dev_tools_manager_delegate_android.cc (right): https://codereview.chromium.org/815983002/diff/400001/chrome/browser/android/dev_tools_manager_delegate_android.cc#newcode343 chrome/browser/android/dev_tools_manager_delegate_android.cc:343: if (top_sites.get()) { On 2015/01/06 11:10:30, Bernhard Bauer wrote: ...
5 years, 11 months ago (2015-01-08 10:22:05 UTC) #32
sdefresne
Some more comments. https://codereview.chromium.org/815983002/diff/500001/chrome/browser/history/top_sites_factory.cc File chrome/browser/history/top_sites_factory.cc (right): https://codereview.chromium.org/815983002/diff/500001/chrome/browser/history/top_sites_factory.cc#newcode1 chrome/browser/history/top_sites_factory.cc:1: // Copyright 2014 The Chromium Authors. ...
5 years, 11 months ago (2015-01-08 10:41:02 UTC) #33
Bernhard Bauer
https://codereview.chromium.org/815983002/diff/500001/chrome/browser/history/top_sites_factory.cc File chrome/browser/history/top_sites_factory.cc (right): https://codereview.chromium.org/815983002/diff/500001/chrome/browser/history/top_sites_factory.cc#newcode12 chrome/browser/history/top_sites_factory.cc:12: using namespace history; On 2015/01/08 10:41:01, sdefresne wrote: > ...
5 years, 11 months ago (2015-01-08 10:45:14 UTC) #34
Jitu( very slow this week)
PTAL https://codereview.chromium.org/815983002/diff/500001/chrome/browser/android/provider/chrome_browser_provider.cc File chrome/browser/android/provider/chrome_browser_provider.cc (right): https://codereview.chromium.org/815983002/diff/500001/chrome/browser/android/provider/chrome_browser_provider.cc#newcode1578 chrome/browser/android/provider/chrome_browser_provider.cc:1578: if (top_sites_.get()) On 2015/01/08 10:22:04, Bernhard Bauer wrote: ...
5 years, 11 months ago (2015-01-12 11:30:09 UTC) #35
Bernhard Bauer
Almost there. https://codereview.chromium.org/815983002/diff/500001/chrome/browser/history/top_sites_factory.cc File chrome/browser/history/top_sites_factory.cc (right): https://codereview.chromium.org/815983002/diff/500001/chrome/browser/history/top_sites_factory.cc#newcode37 chrome/browser/history/top_sites_factory.cc:37: return make_scoped_refptr<history::TopSites>(top_sites); On 2015/01/12 11:30:07, Slow this ...
5 years, 11 months ago (2015-01-12 14:32:23 UTC) #36
Jitu( very slow this week)
PTAL https://codereview.chromium.org/815983002/diff/520001/chrome/browser/history/chrome_history_client.h File chrome/browser/history/chrome_history_client.h (right): https://codereview.chromium.org/815983002/diff/520001/chrome/browser/history/chrome_history_client.h#newcode26 chrome/browser/history/chrome_history_client.h:26: explicit ChromeHistoryClient( On 2015/01/12 14:32:23, Bernhard Bauer wrote: ...
5 years, 11 months ago (2015-01-12 14:55:15 UTC) #37
Bernhard Bauer
https://codereview.chromium.org/815983002/diff/540001/chrome/browser/ui/tabs/tab_strip_model_utils.cc File chrome/browser/ui/tabs/tab_strip_model_utils.cc (right): https://codereview.chromium.org/815983002/diff/540001/chrome/browser/ui/tabs/tab_strip_model_utils.cc#newcode14 chrome/browser/ui/tabs/tab_strip_model_utils.cc:14: history::TopSites& top_sites, Here also.
5 years, 11 months ago (2015-01-12 15:52:50 UTC) #38
sdefresne
This is looking better. I still have some comments, and a suggestion. I think that ...
5 years, 11 months ago (2015-01-12 17:24:24 UTC) #39
loverszhao
On 2015/01/12 17:24:24, sdefresne wrote: > This is looking better. > > I still have ...
5 years, 11 months ago (2015-01-13 07:37:20 UTC) #40
Jitu( very slow this week)
After doing the change and rebase i am getting below errors ../../chrome/browser/ui/browser.cc:1892:42: error: no viable ...
5 years, 11 months ago (2015-01-14 13:03:27 UTC) #41
sdefresne
On 2015/01/14 at 13:03:27, jitendra.ks wrote: > After doing the change and rebase i am ...
5 years, 11 months ago (2015-01-14 21:24:14 UTC) #42
sdefresne
https://codereview.chromium.org/815983002/diff/540001/chrome/test/base/testing_profile.cc File chrome/test/base/testing_profile.cc (right): https://codereview.chromium.org/815983002/diff/540001/chrome/test/base/testing_profile.cc#newcode249 chrome/test/base/testing_profile.cc:249: scoped_refptr<RefcountedKeyedService> BuildTopSites( On 2015/01/12 at 17:24:24, sdefresne wrote: > ...
5 years, 11 months ago (2015-01-14 21:31:48 UTC) #43
Jitu( very slow this week)
On 2015/01/14 21:31:48, sdefresne wrote: > https://codereview.chromium.org/815983002/diff/540001/chrome/test/base/testing_profile.cc > File chrome/test/base/testing_profile.cc (right): > > https://codereview.chromium.org/815983002/diff/540001/chrome/test/base/testing_profile.cc#newcode249 > ...
5 years, 11 months ago (2015-01-19 10:11:37 UTC) #44
sdefresne
> Both the cases i have tried ... it passes most of the cases. > ...
5 years, 11 months ago (2015-01-21 17:57:24 UTC) #45
Jitu( very slow this week)
Thanks sdefresne , As my linux machine was screwed up(some issues) , i was trying ...
5 years, 11 months ago (2015-01-22 05:33:33 UTC) #46
Jitu( very slow this week)
PTAL... Thanks
5 years, 11 months ago (2015-01-22 11:30:12 UTC) #47
sdefresne
This is getting better. I think you'll have something ready once you fix those last ...
5 years, 11 months ago (2015-01-22 19:05:05 UTC) #48
Bernhard Bauer
https://codereview.chromium.org/815983002/diff/580001/chrome/browser/history/top_sites_factory.h File chrome/browser/history/top_sites_factory.h (right): https://codereview.chromium.org/815983002/diff/580001/chrome/browser/history/top_sites_factory.h#newcode39 chrome/browser/history/top_sites_factory.h:39: friend struct DefaultSingletonTraits<TopSitesFactory>; Friend declarations come first in the ...
5 years, 11 months ago (2015-01-22 21:23:27 UTC) #49
Jitu( very slow this week)
PTAL https://codereview.chromium.org/815983002/diff/580001/chrome/browser/autocomplete/zero_suggest_provider_unittest.cc File chrome/browser/autocomplete/zero_suggest_provider_unittest.cc (right): https://codereview.chromium.org/815983002/diff/580001/chrome/browser/autocomplete/zero_suggest_provider_unittest.cc#newcode93 chrome/browser/autocomplete/zero_suggest_provider_unittest.cc:93: void ShutdownOnUIThread() override {} On 2015/01/22 19:05:05, sdefresne ...
5 years, 11 months ago (2015-01-23 11:36:53 UTC) #50
sdefresne
https://codereview.chromium.org/815983002/diff/600001/chrome/browser/history/top_sites_factory.h File chrome/browser/history/top_sites_factory.h (right): https://codereview.chromium.org/815983002/diff/600001/chrome/browser/history/top_sites_factory.h#newcode9 chrome/browser/history/top_sites_factory.h:9: #include "chrome/browser/profiles/profile.h" nit: you can remove this #include and ...
5 years, 11 months ago (2015-01-23 13:25:46 UTC) #51
Jitu( very slow this week)
PTAL https://codereview.chromium.org/815983002/diff/600001/chrome/browser/history/top_sites_factory.h File chrome/browser/history/top_sites_factory.h (right): https://codereview.chromium.org/815983002/diff/600001/chrome/browser/history/top_sites_factory.h#newcode9 chrome/browser/history/top_sites_factory.h:9: #include "chrome/browser/profiles/profile.h" On 2015/01/23 13:25:46, sdefresne wrote: > ...
5 years, 11 months ago (2015-01-23 13:52:52 UTC) #52
sdefresne
lgtm
5 years, 11 months ago (2015-01-23 14:03:43 UTC) #53
Bernhard Bauer
https://codereview.chromium.org/815983002/diff/620001/chrome/browser/history/top_sites_factory.h File chrome/browser/history/top_sites_factory.h (right): https://codereview.chromium.org/815983002/diff/620001/chrome/browser/history/top_sites_factory.h#newcode42 chrome/browser/history/top_sites_factory.h:42: TopSitesFactory(); Constructors and descructors should come before other methods. ...
5 years, 11 months ago (2015-01-23 14:34:07 UTC) #54
Jitu( very slow this week)
PTAL https://codereview.chromium.org/815983002/diff/620001/chrome/browser/history/top_sites_factory.h File chrome/browser/history/top_sites_factory.h (right): https://codereview.chromium.org/815983002/diff/620001/chrome/browser/history/top_sites_factory.h#newcode42 chrome/browser/history/top_sites_factory.h:42: TopSitesFactory(); On 2015/01/23 14:34:06, Bernhard Bauer wrote: > ...
5 years, 11 months ago (2015-01-24 11:48:23 UTC) #56
Bernhard Bauer
lgtm
5 years, 11 months ago (2015-01-26 09:39:35 UTC) #57
Jitu( very slow this week)
Dear Cpu, Can you PTAL.
5 years, 11 months ago (2015-01-26 10:21:39 UTC) #58
Jitu( very slow this week)
Dear Cpu, Can you PTAL.
5 years, 11 months ago (2015-01-26 10:22:01 UTC) #59
sdefresne
On 2015/01/26 at 10:22:01, jitendra.ks wrote: > Dear Cpu, > > Can you PTAL. jitendra.ks: ...
5 years, 11 months ago (2015-01-26 10:38:33 UTC) #60
cpu_(ooo_6.6-7.5)
I am sorry, what in particular should I be looking at? I am not the ...
5 years, 11 months ago (2015-01-26 19:50:28 UTC) #61
sdefresne
jitendra.ks: when selecting OWNERS using "git cl owners" try to pick the OWNERS that is ...
5 years, 11 months ago (2015-01-26 21:28:56 UTC) #64
Elliot Glaysher
I <3 seeing things removed from Profile. lgtm
5 years, 11 months ago (2015-01-26 21:33:04 UTC) #65
Mark P
chrome/browser/autocomplete/... lgtm
5 years, 11 months ago (2015-01-26 22:49:37 UTC) #66
cpu_(ooo_6.6-7.5)
thanks sdefresne, that helps a lot lgtm on - chrome/browser/jumplist_win.cc - chrome/browser/thumbnails/ please wait on ...
5 years, 11 months ago (2015-01-27 00:42:22 UTC) #67
Jay Civelli
https://codereview.chromium.org/815983002/diff/680001/chrome/test/base/testing_profile.h File chrome/test/base/testing_profile.h (right): https://codereview.chromium.org/815983002/diff/680001/chrome/test/base/testing_profile.h#newcode16 chrome/test/base/testing_profile.h:16: #include "components/keyed_service/core/refcounted_keyed_service.h" Is this include needed?
5 years, 11 months ago (2015-01-27 00:58:23 UTC) #68
benwells
c/b/ui/app_list lgtm. i assume that is what you wanted me to look at.
5 years, 11 months ago (2015-01-27 04:20:41 UTC) #69
sdefresne
https://codereview.chromium.org/815983002/diff/680001/chrome/test/base/testing_profile.h File chrome/test/base/testing_profile.h (right): https://codereview.chromium.org/815983002/diff/680001/chrome/test/base/testing_profile.h#newcode16 chrome/test/base/testing_profile.h:16: #include "components/keyed_service/core/refcounted_keyed_service.h" On 2015/01/27 at 00:58:23, Jay Civelli wrote: ...
5 years, 11 months ago (2015-01-27 09:55:27 UTC) #70
sdefresne
On 2015/01/27 at 04:20:41, benwells wrote: > c/b/ui/app_list lgtm. i assume that is what you ...
5 years, 11 months ago (2015-01-27 09:57:46 UTC) #71
Jitu( very slow this week)
Dear Jay Civelli, Fixed the comments for testing_profile.h PTAL https://codereview.chromium.org/815983002/diff/680001/chrome/test/base/testing_profile.h File chrome/test/base/testing_profile.h (right): https://codereview.chromium.org/815983002/diff/680001/chrome/test/base/testing_profile.h#newcode16 chrome/test/base/testing_profile.h:16: ...
5 years, 11 months ago (2015-01-27 10:50:40 UTC) #72
dgozman
chrome/browser/devtools lgtm
5 years, 11 months ago (2015-01-27 11:01:18 UTC) #74
Jay Civelli
LGTM for chrome/test/base
5 years, 11 months ago (2015-01-27 16:33:54 UTC) #75
benwells
c/b/extensions lgtm
5 years, 11 months ago (2015-01-27 21:29:00 UTC) #76
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/815983002/700001
5 years, 11 months ago (2015-01-28 08:58:19 UTC) #78
commit-bot: I haz the power
Committed patchset #19 (id:700001)
5 years, 11 months ago (2015-01-28 09:47:48 UTC) #79
commit-bot: I haz the power
5 years, 11 months ago (2015-01-28 09:48:39 UTC) #80
Message was sent while issue was closed.
Patchset 19 (id:??) landed as
https://crrev.com/30f03390995328247e3642aa5cd2cc9aae781b45
Cr-Commit-Position: refs/heads/master@{#313468}

Powered by Google App Engine
This is Rietveld 408576698