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

Issue 785233002: Enable chrome.gyp:browser on iOS (Closed)

Created:
6 years ago by stuartmorgan
Modified:
6 years ago
Reviewers:
Nico
CC:
chromium-reviews, cbentzel+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Enable chrome.gyp:browser on iOS When unit_tests was turned down on iOS, no chrome.gyp target was added to replace it in all.gyp, so nothing has been exercising that part of the build system on iOS. This adds 'browser', and fixes issues that have crept in since then. Most notably: - Removes chrome_browser_non_android_sources, moving everything that was there to ..._non_mobile_sources - Removes an iOS-only class that was removed downstream, and had rotted upstream. This won't find link issues, but will at least ensure that the gyp files continue working on iOS for as long as they are necessary. BUG=None Committed: https://crrev.com/2d39fdf1a95d374fe589c7f5accff4384f6cf352 Cr-Commit-Position: refs/heads/master@{#307496}

Patch Set 1 #

Total comments: 4

Patch Set 2 : Address comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+135 lines, -177 lines) Patch
M build/all.gyp View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/BUILD.gn View 1 chunk +0 lines, -4 lines 0 comments Download
D chrome/browser/net/spdyproxy/data_reduction_proxy_settings_ios.h View 1 chunk +0 lines, -23 lines 0 comments Download
D chrome/browser/net/spdyproxy/data_reduction_proxy_settings_ios.cc View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/browser/signin/signin_global_error.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/chrome_browser.gypi View 10 chunks +130 lines, -137 lines 0 comments Download
M chrome/chrome_common.gypi View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
stuartmorgan
Doh! It took me a little while to figure out why non_android_sources wasn't breaking iOS ...
6 years ago (2014-12-09 15:28:37 UTC) #2
Nico
lgtm https://codereview.chromium.org/785233002/diff/1/chrome/browser/signin/signin_global_error.cc File chrome/browser/signin/signin_global_error.cc (right): https://codereview.chromium.org/785233002/diff/1/chrome/browser/signin/signin_global_error.cc#newcode96 chrome/browser/signin/signin_global_error.cc:96: // Global errors don't show up in the ...
6 years ago (2014-12-09 16:29:56 UTC) #3
stuartmorgan
https://codereview.chromium.org/785233002/diff/1/chrome/browser/signin/signin_global_error.cc File chrome/browser/signin/signin_global_error.cc (right): https://codereview.chromium.org/785233002/diff/1/chrome/browser/signin/signin_global_error.cc#newcode96 chrome/browser/signin/signin_global_error.cc:96: // Global errors don't show up in the wrench ...
6 years ago (2014-12-09 16:31:26 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/785233002/20001
6 years ago (2014-12-09 16:32:47 UTC) #6
commit-bot: I haz the power
Committed patchset #2 (id:20001)
6 years ago (2014-12-09 17:42:19 UTC) #7
commit-bot: I haz the power
6 years ago (2014-12-09 17:43:06 UTC) #8
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/2d39fdf1a95d374fe589c7f5accff4384f6cf352
Cr-Commit-Position: refs/heads/master@{#307496}

Powered by Google App Engine
This is Rietveld 408576698