Chromium Code Reviews
[email protected] (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(171)

Issue 12211051: Observe devices in NetworkStateHandler (Closed)

Created:
7 years, 10 months ago by stevenjb
Modified:
7 years, 10 months ago
CC:
chromium-reviews, sadrul, ben+watch_chromium.org, gspencer+watch_chromium.org, gauravsh+watch_chromium.org, oshima+watch_chromium.org, stevenjb+watch_chromium.org
Visibility:
Public.

Description

Observe devices in NetworkStateHandler This includes a bit of cleanup to NetworkStateHandler and ShillPropertyHandler. I made ShillServicePropertyObserver support devices, renamed it ShillPropertyObserver, and moved it into ShillPropertyHandler since: a) naming it would be challenging without context b) It doesn't really do much, so if we want something similar (e.g. in NetworkConfigurationHandler) it will be easier just to copy/paste the code we need BUG=161869 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=181254

Patch Set 1 #

Patch Set 2 : Update tests #

Patch Set 3 : . #

Patch Set 4 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+492 lines, -353 lines) Patch
M ash/system/chromeos/network/network_detailed_view.h View 1 chunk +1 line, -1 line 0 comments Download
M ash/system/chromeos/network/network_list_detailed_view_base.h View 1 chunk +1 line, -1 line 0 comments Download
M ash/system/chromeos/network/network_list_detailed_view_base.cc View 1 chunk +1 line, -2 lines 0 comments Download
M ash/system/chromeos/network/network_state_list_detailed_view.h View 2 chunks +1 line, -7 lines 0 comments Download
M ash/system/chromeos/network/network_state_list_detailed_view.cc View 13 chunks +26 lines, -37 lines 0 comments Download
M ash/system/chromeos/network/tray_network.cc View 1 chunk +1 line, -1 line 0 comments Download
M ash/system/chromeos/network/tray_network_state_observer.h View 1 chunk +1 line, -1 line 0 comments Download
M ash/system/chromeos/network/tray_network_state_observer.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M chromeos/chromeos.gyp View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M chromeos/dbus/shill_service_client.cc View 1 3 chunks +10 lines, -2 lines 0 comments Download
M chromeos/network/device_state.h View 2 chunks +2 lines, -0 lines 0 comments Download
M chromeos/network/device_state.cc View 2 chunks +4 lines, -1 line 0 comments Download
M chromeos/network/network_state_handler.h View 10 chunks +38 lines, -32 lines 0 comments Download
M chromeos/network/network_state_handler.cc View 15 chunks +145 lines, -89 lines 0 comments Download
M chromeos/network/network_state_handler_observer.h View 1 chunk +2 lines, -3 lines 0 comments Download
M chromeos/network/network_state_handler_observer.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chromeos/network/network_state_handler_unittest.cc View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M chromeos/network/shill_property_handler.h View 1 8 chunks +40 lines, -20 lines 0 comments Download
M chromeos/network/shill_property_handler.cc View 1 10 chunks +191 lines, -40 lines 0 comments Download
M chromeos/network/shill_property_handler_unittest.cc View 1 7 chunks +22 lines, -30 lines 0 comments Download
D chromeos/network/shill_service_observer.h View 1 chunk +0 lines, -44 lines 0 comments Download
D chromeos/network/shill_service_observer.cc View 1 chunk +0 lines, -33 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
stevenjb
Some cleanup work, mostly to support monitoring Device.Scanning in NetworkStateHandler.
7 years, 10 months ago (2013-02-06 21:07:55 UTC) #1
Greg Spencer (Chromium)
LGTM. FYI: I am using that list argument when the network list changes, so you'll ...
7 years, 10 months ago (2013-02-06 22:21:34 UTC) #2
stevenjb (google-dont-use)
Ah, thanks for the heads-up. Will do. (I removed it because I realized it wasn't ...
7 years, 10 months ago (2013-02-06 23:23:22 UTC) #3
stevenjb
He Greg, I'm putting this in the CQ, because I'd love to have it in ...
7 years, 10 months ago (2013-02-07 04:21:17 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/12211051/13001
7 years, 10 months ago (2013-02-07 04:21:19 UTC) #5
commit-bot: I haz the power
Change committed as 181254
7 years, 10 months ago (2013-02-07 08:14:47 UTC) #6
Vitaly Buka (NO REVIEWS)
On 2013/02/07 08:14:47, I haz the power (commit-bot) wrote: > Change committed as 181254 FYI, ...
7 years, 10 months ago (2013-02-07 08:47:57 UTC) #7
stevenjb (google-dont-use)
7 years, 10 months ago (2013-02-07 15:48:06 UTC) #8
Ugh, sorry, didn't anticipate that. Thanks for the effort.

-Steven



On Thu, Feb 7, 2013 at 12:47 AM, <[email protected]> wrote:

> On 2013/02/07 08:14:47, I haz the power (commit-bot) wrote:
>
>> Change committed as 181254
>>
>
> FYI, We reverted Greg's change today. Then I tried to fix and reapply that
> again, but your change was landed just before that one. So I had to revert
> again.
>
>
https://src.chromium.org/**viewvc/chrome?view=rev&**revision=181257<https://s...
>
>
https://chromiumcodereview.**appspot.com/12211051/<https://chromiumcodereview...
>

Powered by Google App Engine
This is Rietveld 408576698