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

Issue 2202693005: Don't set MAC address randomization when unsupported (Closed)

Created:
4 years, 4 months ago by Eric Caruso
Modified:
4 years, 4 months ago
Reviewers:
stevenjb
CC:
chromium-reviews, stevenjb+watch_chromium.org, oshima+watch_chromium.org, Daniel Kurtz
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Don't set MAC address randomization when unsupported Chrome currently tries to set MAC address randomization on the WiFi device whenever the it detects changes in the device list, but this means it spams shill with calls and floods shill's log with errors if the feature is unsupported. Instead, stop trying to toggle the property when this happens. BUG=630012 R=stevenjb@chromium.org Committed: https://crrev.com/ad8797fd5b55ee3aff6bf06d2a91633fc294f079 Cr-Commit-Position: refs/heads/master@{#409388}

Patch Set 1 #

Total comments: 1

Patch Set 2 : use weak ptr #

Unified diffs Side-by-side diffs Delta from patch set Stats (+29 lines, -8 lines) Patch
M chromeos/network/network_device_handler_impl.h View 1 2 chunks +8 lines, -1 line 0 comments Download
M chromeos/network/network_device_handler_impl.cc View 1 3 chunks +21 lines, -7 lines 0 comments Download

Messages

Total messages: 10 (3 generated)
Eric Caruso
PTAL, thanks.
4 years, 4 months ago (2016-08-02 22:33:52 UTC) #1
stevenjb
https://codereview.chromium.org/2202693005/diff/1/chromeos/network/network_device_handler_impl.cc File chromeos/network/network_device_handler_impl.cc (right): https://codereview.chromium.org/2202693005/diff/1/chromeos/network/network_device_handler_impl.cc#newcode591 chromeos/network/network_device_handler_impl.cc:591: base::Unretained(this))); We need to use a weak ptr here, ...
4 years, 4 months ago (2016-08-02 22:42:34 UTC) #2
Eric Caruso
PTAL, thanks.
4 years, 4 months ago (2016-08-02 23:03:29 UTC) #3
stevenjb
LGTM, thanks for fixing this!
4 years, 4 months ago (2016-08-02 23:06:43 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/2202693005/20001
4 years, 4 months ago (2016-08-02 23:08:33 UTC) #7
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 4 months ago (2016-08-03 00:04:41 UTC) #8
commit-bot: I haz the power
4 years, 4 months ago (2016-08-03 00:07:29 UTC) #10
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/ad8797fd5b55ee3aff6bf06d2a91633fc294f079
Cr-Commit-Position: refs/heads/master@{#409388}

Powered by Google App Engine
This is Rietveld 408576698