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

Issue 530193004: Fix wifi_component build with 10.9+ SDK (Closed)

Created:
6 years, 3 months ago by Jiang Jiang
Modified:
6 years, 3 months ago
Reviewers:
Robert Sesek, mef
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Fix wifi_component build with 10.9+ SDK Use forward declaration to build on both 10.6 and 10.9 SDK and runtime checking to make sure we don't call the wrong API. BUG=390212 Committed: https://crrev.com/6acb57597d5d18ab9d0e2954993863a8e2848774 Cr-Commit-Position: refs/heads/master@{#293911}

Patch Set 1 #

Total comments: 8

Patch Set 2 : Move forward declare to base and fix style issues #

Total comments: 6

Patch Set 3 : Add missing comment after #endif #

Total comments: 6

Patch Set 4 : Combine 10.9 forward declare section #

Patch Set 5 : Update comments #

Patch Set 6 : Remove one more "we" #

Patch Set 7 : Use correct notification #

Total comments: 2

Patch Set 8 : Remove runtime checking in favor of simpler code #

Total comments: 4

Patch Set 9 : Fix remaining issues #

Unified diffs Side-by-side diffs Delta from patch set Stats (+93 lines, -3 lines) Patch
M base/mac/sdk_forward_declarations.h View 1 2 3 4 5 6 7 8 1 chunk +45 lines, -0 lines 0 comments Download
M components/wifi/wifi_service_mac.mm View 1 2 3 4 5 6 7 8 3 chunks +48 lines, -3 lines 0 comments Download

Messages

Total messages: 50 (15 generated)
Jiang Jiang
6 years, 3 months ago (2014-09-04 15:04:36 UTC) #2
Robert Sesek
Thanks for doing this! It's less tricky than I thought it was, though they did ...
6 years, 3 months ago (2014-09-04 15:13:30 UTC) #4
Jiang Jiang
https://codereview.chromium.org/530193004/diff/1/components/wifi/wifi_service_mac.mm File components/wifi/wifi_service_mac.mm (right): https://codereview.chromium.org/530193004/diff/1/components/wifi/wifi_service_mac.mm#newcode24 components/wifi/wifi_service_mac.mm:24: #if !defined(MAC_OS_X_VERSION_10_9) || \ On 2014/09/04 15:13:30, rsesek wrote: ...
6 years, 3 months ago (2014-09-04 15:17:50 UTC) #5
Robert Sesek
https://codereview.chromium.org/530193004/diff/20001/base/mac/sdk_forward_declarations.h File base/mac/sdk_forward_declarations.h (right): https://codereview.chromium.org/530193004/diff/20001/base/mac/sdk_forward_declarations.h#newcode279 base/mac/sdk_forward_declarations.h:279: #if !defined(MAC_OS_X_VERSION_10_9) || \ There's already a 10.9 section ...
6 years, 3 months ago (2014-09-04 15:24:35 UTC) #6
mef
https://codereview.chromium.org/530193004/diff/40001/base/mac/sdk_forward_declarations.h File base/mac/sdk_forward_declarations.h (right): https://codereview.chromium.org/530193004/diff/40001/base/mac/sdk_forward_declarations.h#newcode277 base/mac/sdk_forward_declarations.h:277: // build with SDKs from 10.6 to 10.9 we ...
6 years, 3 months ago (2014-09-04 15:31:34 UTC) #7
Jiang Jiang
https://codereview.chromium.org/530193004/diff/20001/base/mac/sdk_forward_declarations.h File base/mac/sdk_forward_declarations.h (right): https://codereview.chromium.org/530193004/diff/20001/base/mac/sdk_forward_declarations.h#newcode279 base/mac/sdk_forward_declarations.h:279: #if !defined(MAC_OS_X_VERSION_10_9) || \ On 2014/09/04 15:24:35, rsesek wrote: ...
6 years, 3 months ago (2014-09-04 15:32:17 UTC) #8
Robert Sesek
https://codereview.chromium.org/530193004/diff/20001/base/mac/sdk_forward_declarations.h File base/mac/sdk_forward_declarations.h (right): https://codereview.chromium.org/530193004/diff/20001/base/mac/sdk_forward_declarations.h#newcode279 base/mac/sdk_forward_declarations.h:279: #if !defined(MAC_OS_X_VERSION_10_9) || \ On 2014/09/04 15:32:17, Jiang wrote: ...
6 years, 3 months ago (2014-09-04 15:34:43 UTC) #9
Jiang Jiang
https://codereview.chromium.org/530193004/diff/40001/base/mac/sdk_forward_declarations.h File base/mac/sdk_forward_declarations.h (right): https://codereview.chromium.org/530193004/diff/40001/base/mac/sdk_forward_declarations.h#newcode277 base/mac/sdk_forward_declarations.h:277: // build with SDKs from 10.6 to 10.9 we ...
6 years, 3 months ago (2014-09-04 16:04:21 UTC) #10
Robert Sesek
LGTM
6 years, 3 months ago (2014-09-04 16:31:15 UTC) #11
mef
lgtm
6 years, 3 months ago (2014-09-04 17:14:16 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jiangj@opera.com/530193004/100001
6 years, 3 months ago (2014-09-04 17:50:09 UTC) #14
Jiang Jiang
On 2014/09/04 17:50:09, I haz the power (commit-bot) wrote: > CQ is trying da patch. ...
6 years, 3 months ago (2014-09-04 19:14:26 UTC) #15
mef
On 2014/09/04 19:14:26, Jiang wrote: > On 2014/09/04 17:50:09, I haz the power (commit-bot) wrote: ...
6 years, 3 months ago (2014-09-04 19:20:18 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/51700) mac_chromium_compile_dbg on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg/builds/12323) mac_chromium_rel_swarming ...
6 years, 3 months ago (2014-09-04 19:59:56 UTC) #18
Jiang Jiang
On 2014/09/04 19:14:26, Jiang wrote: > On 2014/09/04 17:50:09, I haz the power (commit-bot) wrote: ...
6 years, 3 months ago (2014-09-05 10:06:43 UTC) #19
Jiang Jiang
On 2014/09/04 19:20:18, mef wrote: > On 2014/09/04 19:14:26, Jiang wrote: > > On 2014/09/04 ...
6 years, 3 months ago (2014-09-05 17:47:36 UTC) #20
mef
https://codereview.chromium.org/530193004/diff/120001/components/wifi/wifi_service_mac.mm File components/wifi/wifi_service_mac.mm (right): https://codereview.chromium.org/530193004/diff/120001/components/wifi/wifi_service_mac.mm#newcode408 components/wifi/wifi_service_mac.mm:408: NSBundle* bundle = [NSBundle If anything I'd move this ...
6 years, 3 months ago (2014-09-05 17:56:23 UTC) #21
Jiang Jiang
On 2014/09/05 17:56:23, mef wrote: > https://codereview.chromium.org/530193004/diff/120001/components/wifi/wifi_service_mac.mm > File components/wifi/wifi_service_mac.mm (right): > > https://codereview.chromium.org/530193004/diff/120001/components/wifi/wifi_service_mac.mm#newcode408 > ...
6 years, 3 months ago (2014-09-05 18:04:54 UTC) #22
Jiang Jiang
On 2014/09/05 18:04:54, Jiang wrote: > On 2014/09/05 17:56:23, mef wrote: > > > https://codereview.chromium.org/530193004/diff/120001/components/wifi/wifi_service_mac.mm ...
6 years, 3 months ago (2014-09-05 18:08:34 UTC) #23
Robert Sesek
LGTM https://codereview.chromium.org/530193004/diff/140001/base/mac/sdk_forward_declarations.h File base/mac/sdk_forward_declarations.h (right): https://codereview.chromium.org/530193004/diff/140001/base/mac/sdk_forward_declarations.h#newcode285 base/mac/sdk_forward_declarations.h:285: #else // !MAC_OS_X_VERSION_10_9
6 years, 3 months ago (2014-09-05 18:18:43 UTC) #24
mef
I'm glad it is settled to less ugly hack. :) Still LGTM. https://codereview.chromium.org/530193004/diff/140001/components/wifi/wifi_service_mac.mm File components/wifi/wifi_service_mac.mm ...
6 years, 3 months ago (2014-09-05 18:22:50 UTC) #25
Jiang Jiang
https://codereview.chromium.org/530193004/diff/140001/base/mac/sdk_forward_declarations.h File base/mac/sdk_forward_declarations.h (right): https://codereview.chromium.org/530193004/diff/140001/base/mac/sdk_forward_declarations.h#newcode285 base/mac/sdk_forward_declarations.h:285: #else On 2014/09/05 18:18:43, rsesek wrote: > // !MAC_OS_X_VERSION_10_9 ...
6 years, 3 months ago (2014-09-06 06:42:57 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jiangj@opera.com/530193004/140001
6 years, 3 months ago (2014-09-06 06:45:20 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jiangj@opera.com/530193004/160001
6 years, 3 months ago (2014-09-08 08:57:35 UTC) #35
commit-bot: I haz the power
Exceeded time limit waiting for builds to trigger.
6 years, 3 months ago (2014-09-08 10:58:14 UTC) #37
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jiangj@opera.com/530193004/160001
6 years, 3 months ago (2014-09-08 14:53:56 UTC) #39
commit-bot: I haz the power
Exceeded time limit waiting for builds to trigger.
6 years, 3 months ago (2014-09-08 16:54:32 UTC) #41
Jiang Jiang
On 2014/09/08 16:54:32, I haz the power (commit-bot) wrote: > Exceeded time limit waiting for ...
6 years, 3 months ago (2014-09-08 17:03:11 UTC) #42
Robert Sesek
On 2014/09/08 17:03:11, Jiang wrote: > On 2014/09/08 16:54:32, I haz the power (commit-bot) wrote: ...
6 years, 3 months ago (2014-09-08 17:05:13 UTC) #43
Jiang Jiang
On 2014/09/08 17:05:13, rsesek wrote: > On 2014/09/08 17:03:11, Jiang wrote: > > On 2014/09/08 ...
6 years, 3 months ago (2014-09-09 08:47:09 UTC) #44
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jiangj@opera.com/530193004/160001
6 years, 3 months ago (2014-09-09 08:48:41 UTC) #46
commit-bot: I haz the power
Committed patchset #9 (id:160001) as bac6205d2a0dc31dfd2040fe6f58416d0a7905af
6 years, 3 months ago (2014-09-09 09:58:18 UTC) #47
pdr.
On 2014/09/09 09:58:18, I haz the power (commit-bot) wrote: > Committed patchset #9 (id:160001) as ...
6 years, 3 months ago (2014-09-09 21:41:11 UTC) #48
Jiang Jiang
On 2014/09/09 21:41:11, pdr wrote: > On 2014/09/09 09:58:18, I haz the power (commit-bot) wrote: ...
6 years, 3 months ago (2014-09-09 21:53:32 UTC) #49
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 03:52:29 UTC) #50
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/6acb57597d5d18ab9d0e2954993863a8e2848774
Cr-Commit-Position: refs/heads/master@{#293911}

Powered by Google App Engine
This is Rietveld 408576698