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

Issue 10388239: Make NetworkProfileBubble not use BrowserList::GetLastActive() as much. Instead pass it a profile a… (Closed)

Created:
8 years, 7 months ago by Ben Goodger (Google)
Modified:
8 years, 7 months ago
Reviewers:
sky
CC:
chromium-reviews, robertshield, Aaron Boodman, rginda+watch_chromium.org, mihaip-chromium-reviews_chromium.org, kkania
Visibility:
Public.

Description

Make NetworkProfileBubble not use BrowserList::GetLastActive() as much. Instead pass it a profile and a PageNavigator. This also required me to make BrowserList::Observer methods take a non-const Browser*. It was pretty silly they were const to begin with, since you pretty much only ever get a Browser* to do something to it. http://crbug.com/129187 TEST=none Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=138486

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+93 lines, -78 lines) Patch
M chrome/browser/automation/testing_automation_provider.h View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/automation/testing_automation_provider.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/bundle_installer.h View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/bundle_installer.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/extension_browser_event_router.h View 3 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/extensions/extension_browser_event_router.cc View 7 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/managed_mode.h View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/managed_mode.cc View 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/profiles/profile_manager.h View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/profiles/profile_manager.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/protector/settings_change_global_error.h View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/protector/settings_change_global_error.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/browser_list.h View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/cocoa/profile_menu_controller.mm View 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/browser/ui/startup/startup_browser_creator_browsertest.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/views/network_profile_bubble.h View 3 chunks +13 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/network_profile_bubble.cc View 5 chunks +31 lines, -27 lines 1 comment Download

Messages

Total messages: 5 (0 generated)
Ben Goodger (Google)
8 years, 7 months ago (2012-05-22 23:36:15 UTC) #1
sky
LGTM http://codereview.chromium.org/10388239/diff/1/chrome/browser/ui/views/network_profile_bubble.cc File chrome/browser/ui/views/network_profile_bubble.cc (right): http://codereview.chromium.org/10388239/diff/1/chrome/browser/ui/views/network_profile_bubble.cc#newcode20 chrome/browser/ui/views/network_profile_bubble.cc:20: #include "chrome/browser/ui/browser_list.h" Can you remove this include now?
8 years, 7 months ago (2012-05-23 00:03:06 UTC) #2
Ben Goodger (Google)
On 2012/05/23 00:03:06, sky wrote: > LGTM > > http://codereview.chromium.org/10388239/diff/1/chrome/browser/ui/views/network_profile_bubble.cc > File chrome/browser/ui/views/network_profile_bubble.cc (right): > ...
8 years, 7 months ago (2012-05-23 02:06:13 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ben@chromium.org/10388239/1
8 years, 7 months ago (2012-05-23 02:11:01 UTC) #4
commit-bot: I haz the power
8 years, 7 months ago (2012-05-23 02:11:16 UTC) #5
Presubmit check for 10388239-1 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/profiles

Presubmit checks took 5.9s to calculate.

Powered by Google App Engine
This is Rietveld 408576698