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

Issue 2833273002: Geolocation cleanup: use unique_ptr<>s and rename New{WlanApi/PollingPolicy} to CreateBla() (Closed)

Created:
3 years, 8 months ago by mcasas
Modified:
3 years, 8 months ago
CC:
chromium-reviews, mac-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Geolocation cleanup: use unique_ptr<>s and rename New{WlanApi/PollingPolicy} to CreateBla() Cleanup to renamed NewWlanApi to CreateWlanApi and NewPollingPolicy() to CreatePollingPolicy(), and made them return unique_ptr<> ISO a naked ptrs. Also in device/geolocation/wifi_data_provider_win.cc removed {} for one-line bodies and also had to change the signaure of two internal Create() methods (and make ctors public to be accessible by base::MakeUnique<>). BUG=714348 Review-Url: https://codereview.chromium.org/2833273002 Cr-Commit-Position: refs/heads/master@{#466771} Committed: https://chromium.googlesource.com/chromium/src/+/b477ea757773359fb6d25a643815ded1c0660909

Patch Set 1 : #

Total comments: 4

Patch Set 2 : reillyg@s nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+88 lines, -95 lines) Patch
M device/geolocation/wifi_data_provider_common.h View 1 chunk +2 lines, -2 lines 0 comments Download
M device/geolocation/wifi_data_provider_common.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M device/geolocation/wifi_data_provider_common_unittest.cc View 1 chunk +5 lines, -3 lines 0 comments Download
M device/geolocation/wifi_data_provider_linux.h View 1 chunk +4 lines, -5 lines 0 comments Download
M device/geolocation/wifi_data_provider_linux.cc View 5 chunks +19 lines, -17 lines 0 comments Download
M device/geolocation/wifi_data_provider_linux_unittest.cc View 1 1 chunk +2 lines, -3 lines 0 comments Download
M device/geolocation/wifi_data_provider_mac.h View 1 chunk +3 lines, -3 lines 0 comments Download
M device/geolocation/wifi_data_provider_mac.mm View 2 chunks +7 lines, -5 lines 0 comments Download
M device/geolocation/wifi_data_provider_win.h View 1 chunk +3 lines, -3 lines 0 comments Download
M device/geolocation/wifi_data_provider_win.cc View 1 13 chunks +41 lines, -52 lines 0 comments Download

Messages

Total messages: 30 (25 generated)
mcasas
reillyg@ PTAL
3 years, 8 months ago (2017-04-24 02:34:46 UTC) #11
Reilly Grant (use Gerrit)
lgtm with nits https://codereview.chromium.org/2833273002/diff/140001/device/geolocation/wifi_data_provider_linux_unittest.cc File device/geolocation/wifi_data_provider_linux_unittest.cc (right): https://codereview.chromium.org/2833273002/diff/140001/device/geolocation/wifi_data_provider_linux_unittest.cc#newcode96 device/geolocation/wifi_data_provider_linux_unittest.cc:96: ASSERT_TRUE(wlan_api_.get()); ASSERT_TRUE(wlan_api_); https://codereview.chromium.org/2833273002/diff/140001/device/geolocation/wifi_data_provider_win.cc File device/geolocation/wifi_data_provider_win.cc (right): ...
3 years, 8 months ago (2017-04-24 19:42:33 UTC) #19
mcasas
https://codereview.chromium.org/2833273002/diff/140001/device/geolocation/wifi_data_provider_linux_unittest.cc File device/geolocation/wifi_data_provider_linux_unittest.cc (right): https://codereview.chromium.org/2833273002/diff/140001/device/geolocation/wifi_data_provider_linux_unittest.cc#newcode96 device/geolocation/wifi_data_provider_linux_unittest.cc:96: ASSERT_TRUE(wlan_api_.get()); On 2017/04/24 19:42:33, Reilly Grant wrote: > ASSERT_TRUE(wlan_api_); ...
3 years, 8 months ago (2017-04-24 19:47:35 UTC) #21
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/2833273002/160001
3 years, 8 months ago (2017-04-24 21:20:14 UTC) #27
commit-bot: I haz the power
3 years, 8 months ago (2017-04-24 21:25:01 UTC) #30
Message was sent while issue was closed.
Committed patchset #2 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/b477ea757773359fb6d25a643815...

Powered by Google App Engine
This is Rietveld 408576698