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

Issue 2735973003: Make BrowserCoordinator use a Browser instead of a BrowserState (Closed)

Created:
3 years, 9 months ago by lpromero
Modified:
3 years, 9 months ago
CC:
chromium-reviews, marq+scrutinize_chromium.org, lpromero+watch_chromium.org, ios-reviews+clean_chromium.org, ios-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make BrowserCoordinator use a Browser instead of a BrowserState BUG=none R=sdefresne@chromium.org Review-Url: https://codereview.chromium.org/2735973003 Cr-Commit-Position: refs/heads/master@{#455780} Committed: https://chromium.googlesource.com/chromium/src/+/b4478d5798a58d39e4975f14122ed064e0ca4466

Patch Set 1 #

Total comments: 3

Patch Set 2 : Remove unused import #

Unified diffs Side-by-side diffs Delta from patch set Stats (+26 lines, -16 lines) Patch
M ios/clean/chrome/app/steps/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M ios/clean/chrome/app/steps/tab_grid_coordinator+application_step.mm View 1 2 chunks +4 lines, -1 line 0 comments Download
M ios/clean/chrome/browser/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M ios/clean/chrome/browser/browser_coordinator.h View 2 chunks +4 lines, -6 lines 0 comments Download
M ios/clean/chrome/browser/browser_coordinator.mm View 2 chunks +2 lines, -2 lines 0 comments Download
M ios/clean/chrome/browser/browser_coordinator+internal.h View 1 chunk +1 line, -1 line 0 comments Download
M ios/clean/chrome/browser/ui/settings/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M ios/clean/chrome/browser/ui/settings/settings_coordinator.mm View 2 chunks +5 lines, -2 lines 0 comments Download
M ios/clean/chrome/browser/ui/tab_grid/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm View 3 chunks +6 lines, -4 lines 0 comments Download

Messages

Total messages: 15 (9 generated)
lpromero
3 years, 9 months ago (2017-03-07 18:08:05 UTC) #1
lpromero
https://codereview.chromium.org/2735973003/diff/1/ios/clean/chrome/app/steps/tab_grid_coordinator+application_step.mm File ios/clean/chrome/app/steps/tab_grid_coordinator+application_step.mm (right): https://codereview.chromium.org/2735973003/diff/1/ios/clean/chrome/app/steps/tab_grid_coordinator+application_step.mm#newcode50 ios/clean/chrome/app/steps/tab_grid_coordinator+application_step.mm:50: BrowserList::FromBrowserState(state.browserState)->CreateNewBrowser(); This should be the only place we create ...
3 years, 9 months ago (2017-03-07 18:10:43 UTC) #4
rohitrao (ping after 24h)
lgtm https://codereview.chromium.org/2735973003/diff/1/ios/clean/chrome/app/steps/tab_grid_coordinator+application_step.mm File ios/clean/chrome/app/steps/tab_grid_coordinator+application_step.mm (right): https://codereview.chromium.org/2735973003/diff/1/ios/clean/chrome/app/steps/tab_grid_coordinator+application_step.mm#newcode11 ios/clean/chrome/app/steps/tab_grid_coordinator+application_step.mm:11: #include "base/memory/ptr_util.h" Is this needed?
3 years, 9 months ago (2017-03-09 15:10:23 UTC) #8
lpromero
https://codereview.chromium.org/2735973003/diff/1/ios/clean/chrome/app/steps/tab_grid_coordinator+application_step.mm File ios/clean/chrome/app/steps/tab_grid_coordinator+application_step.mm (right): https://codereview.chromium.org/2735973003/diff/1/ios/clean/chrome/app/steps/tab_grid_coordinator+application_step.mm#newcode11 ios/clean/chrome/app/steps/tab_grid_coordinator+application_step.mm:11: #include "base/memory/ptr_util.h" On 2017/03/09 15:10:23, rohitrao wrote: > Is ...
3 years, 9 months ago (2017-03-09 16:31:27 UTC) #11
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/2735973003/20001
3 years, 9 months ago (2017-03-09 16:31:57 UTC) #12
commit-bot: I haz the power
3 years, 9 months ago (2017-03-09 17:27:17 UTC) #15
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/b4478d5798a58d39e4975f14122e...

Powered by Google App Engine
This is Rietveld 408576698