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

Issue 551373005: Enable ash shell and unit tests in GN build. (Closed)

Created:
6 years, 3 months ago by brettw
Modified:
6 years, 3 months ago
CC:
chromium-reviews, tdanderson+overview_chromium.org, kalyank, sadrul, ben+ash_chromium.org, tfarina
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Enable ash shell and unit tests in GN build. Update ash gyp files. There were some obsolete 'sources!' entries and a non-test file accidentally in the ash unittests sources. workspace_backdrop_delegate.cc was duplicated in both ash and the unit tests target. Apparetcly this was done to make the component build work because it didn't have the export designation. The GN build detected this because it uses source_set instead of static_library which caught the duplicate symbol. This adds the export designation, removes the duplicate, and also adds ash_export.h to the sources list. There were some warnings about superfluous const usage that surfaced. I assume this file isn't being compiled with chromium_code in the GYP build. Committed: https://crrev.com/ea76f5f5c3d6e0f73876d040265ad121346e9f3d Cr-Commit-Position: refs/heads/master@{#294030}

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : non-exported base for windows #

Unified diffs Side-by-side diffs Delta from patch set Stats (+230 lines, -18 lines) Patch
M ash/BUILD.gn View 1 4 chunks +6 lines, -10 lines 0 comments Download
M ash/ash.gyp View 1 2 3 chunks +1 line, -4 lines 0 comments Download
M ash/display/display_controller_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M ash/wm/maximize_mode/workspace_backdrop_delegate.h View 1 2 3 2 chunks +5 lines, -2 lines 0 comments Download
M ash/wm/overview/window_selector_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
A ui/views/examples/BUILD.gn View 1 chunk +154 lines, -0 lines 0 comments Download
M ui/views/examples/examples.gyp View 8 chunks +8 lines, -0 lines 0 comments Download
A ui/views_content_client/BUILD.gn View 1 chunk +52 lines, -0 lines 0 comments Download
M ui/views_content_client/views_content_client.gyp View 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (4 generated)
brettw
6 years, 3 months ago (2014-09-09 17:04:31 UTC) #2
Ben Goodger (Google)
lgtm
6 years, 3 months ago (2014-09-09 17:05:30 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/brettw@chromium.org/551373005/40001
6 years, 3 months ago (2014-09-09 19:58:24 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/brettw@chromium.org/551373005/60001
6 years, 3 months ago (2014-09-09 20:58:24 UTC) #8
commit-bot: I haz the power
Committed patchset #4 (id:60001) as 519597424abbe8f828779a5d5d0bb16fab288670
6 years, 3 months ago (2014-09-09 23:16:00 UTC) #9
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 03:56:21 UTC) #10
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/ea76f5f5c3d6e0f73876d040265ad121346e9f3d
Cr-Commit-Position: refs/heads/master@{#294030}

Powered by Google App Engine
This is Rietveld 408576698