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

Issue 2890193003: Open Most Visited items (Closed)

Created:
3 years, 7 months ago by gambard
Modified:
3 years, 6 months ago
CC:
chromium-reviews, marq+watch_chromium.org, ios-reviews+chrome_chromium.org, noyau+watch_chromium.org, ios-reviews_chromium.org, pkl (ping after 24h if needed)
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Open Most Visited items Open the Most Visited items when tapped and log the action. BUG=724427 Review-Url: https://codereview.chromium.org/2890193003 Cr-Commit-Position: refs/heads/master@{#477262} Committed: https://chromium.googlesource.com/chromium/src/+/d9ba02556dbc40c2059778aaab0f3e1dabc1acd0

Patch Set 1 #

Patch Set 2 : Add myself as favicon owner #

Total comments: 2

Patch Set 3 : Fix gn includes #

Patch Set 4 : Change function name #

Patch Set 5 : Change function name again #

Patch Set 6 : Fix showcase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+105 lines, -56 lines) Patch
M ios/chrome/browser/content_suggestions/BUILD.gn View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm View 3 chunks +23 lines, -0 lines 0 comments Download
M ios/chrome/browser/content_suggestions/mediator_util.mm View 1 chunk +1 line, -0 lines 0 comments Download
M ios/chrome/browser/ui/content_suggestions/cells/BUILD.gn View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_most_visited_item.h View 1 chunk +7 lines, -4 lines 0 comments Download
M ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_most_visited_item.mm View 2 chunks +12 lines, -0 lines 0 comments Download
M ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_most_visited_item_unittest.mm View 1 chunk +2 lines, -1 line 0 comments Download
M ios/chrome/browser/ui/content_suggestions/content_suggestions_commands.h View 1 chunk +3 lines, -0 lines 0 comments Download
M ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm View 1 2 3 4 2 chunks +10 lines, -1 line 0 comments Download
A ios/chrome/browser/ui/favicon/OWNERS View 1 1 chunk +1 line, -0 lines 0 comments Download
M ios/chrome/browser/ui/favicon/favicon_attributes.h View 1 chunk +17 lines, -23 lines 0 comments Download
M ios/chrome/browser/ui/favicon/favicon_attributes.mm View 2 chunks +19 lines, -22 lines 0 comments Download
M ios/chrome/browser/ui/favicon/favicon_attributes_provider.mm View 1 chunk +6 lines, -4 lines 0 comments Download
M ios/showcase/content_suggestions/sc_content_suggestions_data_source.mm View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 19 (11 generated)
gambard
PTAL.
3 years, 7 months ago (2017-05-19 11:51:12 UTC) #2
marq (ping after 24h)
LGTM with one very optional nit. Nice work. https://codereview.chromium.org/2890193003/diff/20001/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm File ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm (right): https://codereview.chromium.org/2890193003/diff/20001/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm#newcode278 ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm:278: - ...
3 years, 6 months ago (2017-06-02 08:25:47 UTC) #3
gambard
https://codereview.chromium.org/2890193003/diff/20001/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm File ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm (right): https://codereview.chromium.org/2890193003/diff/20001/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm#newcode278 ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm:278: - (void)blurOmnibox { On 2017/06/02 08:25:47, marq (ping after ...
3 years, 6 months ago (2017-06-02 13:26:59 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2890193003/60001
3 years, 6 months ago (2017-06-06 09:42:47 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2890193003/80001
3 years, 6 months ago (2017-06-06 09:52:46 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/builds/231377)
3 years, 6 months ago (2017-06-06 10:01:59 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2890193003/100001
3 years, 6 months ago (2017-06-06 10:05:48 UTC) #16
commit-bot: I haz the power
3 years, 6 months ago (2017-06-06 12:01:51 UTC) #19
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/d9ba02556dbc40c2059778aaab0f...

Powered by Google App Engine
This is Rietveld 408576698