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

Issue 12258019: Make BrowserList::(Add|Remove)Observer add the observer to every desktop's browser list. (Closed)

Created:
7 years, 10 months ago by gab
Modified:
7 years, 10 months ago
Reviewers:
sky
CC:
chromium-reviews, robertshield, kkania
Visibility:
Public.

Description

Make BrowserList::(Add|Remove)Observer add the observer to every desktop's browser list. Also make it impossible to call AddObserver on an individual list from outside BrowserList. This is to make sure the paradigm becomes that the consumer code is in charge of checking for HostDesktopType (should he care) upon receiving an event; allowing AddObserver to be called on an individual list would put burden on the reader to understand whether the events are coming from multiple browser lists or a single one (which seems unecessary for now). BUG=129187 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=182384

Patch Set 1 #

Patch Set 2 : move friend to top of private section (since I need it for other methods in following CL) #

Patch Set 3 : Need to friend the methods directly since they are static (otherwise linux_clang doesn't like it) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -15 lines) Patch
M chrome/browser/automation/testing_automation_provider.cc View 3 chunks +3 lines, -6 lines 0 comments Download
M chrome/browser/ui/browser_list.h View 1 chunk +4 lines, -3 lines 0 comments Download
M chrome/browser/ui/browser_list.cc View 2 chunks +11 lines, -2 lines 0 comments Download
M chrome/browser/ui/browser_list_impl.h View 1 2 3 chunks +9 lines, -4 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
gab
Scott, please take a look. Thanks! Gab
7 years, 10 months ago (2013-02-13 21:00:40 UTC) #1
sky
LGTM
7 years, 10 months ago (2013-02-13 22:18:52 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/gab@chromium.org/12258019/7001
7 years, 10 months ago (2013-02-13 22:31:34 UTC) #3
commit-bot: I haz the power
Retried try job too often on linux_chromeos for step(s) aura_unittests, base_unittests, browser_tests, cacheinvalidation_unittests, check_deps, chromeos_unittests, ...
7 years, 10 months ago (2013-02-13 23:00:12 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/gab@chromium.org/12258019/7001
7 years, 10 months ago (2013-02-13 23:21:58 UTC) #5
commit-bot: I haz the power
7 years, 10 months ago (2013-02-14 04:18:52 UTC) #6
Message was sent while issue was closed.
Change committed as 182384

Powered by Google App Engine
This is Rietveld 408576698