New issue
Advanced search Search tips

Issue 611324 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

MacViews: GN: Get a gn compile green with mac_views_browser = true

Project Member Reported by tapted@chromium.org, May 12 2016

Issue description

Chrome Version       : 52.0.2733.0
OS Version: OS X 10.11.4

E.g.

gn args out/gn_Release

mac_views_browser = false
is_debug = true
is_component_build = false


ninja -C out/gn_Release chrome all

Goal: No errors \o/

 

Comment 1 by tapted@chromium.org, May 12 2016

Components: Build
Labels: Proj-MacViews Proj-GN-Migration
Started in https://codereview.chromium.org/1972843002

status: chrome compiles \o/ with mac_views_browser = true 

(but, as I write this, not with = false - doh. Also lots of things need linting).

Comment 2 by rsesek@chromium.org, May 13 2016

Thanks for getting started on this.

Since mac_views_browser isn't a shipped config at the moment, if you get the build working, I think you can make the GN version the definitive source of truth for mac_views_browser and stop supporting GYP.
Project Member

Comment 3 by bugdroid1@chromium.org, May 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/64351f1c48aebf9167b6b1a0cfcec6a1779fbffa

commit 64351f1c48aebf9167b6b1a0cfcec6a1779fbffa
Author: tapted <tapted@chromium.org>
Date: Tue May 17 06:10:50 2016

MacViews GN: Get chrome compiling with mac_views_browser = true

Three things only used by Mac+Cocoa (not mac_views_browser), but ignored
when linking with gyp since they were in the libbrowser.a library and
symbols in libbrowser_ui.a took precedence:
* Moves DownloadDangerPromptImpl from c/b/download to
c/b/ui/cocoa/download
* Moves chrome::ShowCreateChromeAppShortcutsDialog from
c/b/web_applications to c/b/ui/cocoa
* proximity_auth_error_bubble.cc had stubs only used by Cocoa.

Remove immersive_mode_controller_factory_mac.cc - Cocoa doesn't use it,
and mac_views_browser has an implementation in c/b/ui/views

gyp doesn't automatically strip _aura files -- move some aura-only stuff
into chrome_browser_ui_aura_sources. (x11_panel_resizer is Linux only
but was actually being compiled on all aura platforms - fix that).

chrome_browser_ui_views_*non_mac*_sources had some _mac stuff used by
mac_views_browser=1 builds, move these to
chrome_browser_ui_views_mac_experimental_sources to avoid confusion.

BUG= 611324 

Review-Url: https://codereview.chromium.org/1972843002
Cr-Commit-Position: refs/heads/master@{#394068}

[modify] https://crrev.com/64351f1c48aebf9167b6b1a0cfcec6a1779fbffa/chrome/browser/download/download_danger_prompt.cc
[modify] https://crrev.com/64351f1c48aebf9167b6b1a0cfcec6a1779fbffa/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/64351f1c48aebf9167b6b1a0cfcec6a1779fbffa/chrome/browser/ui/cocoa/create_application_shortcut_cocoa.mm
[add] https://crrev.com/64351f1c48aebf9167b6b1a0cfcec6a1779fbffa/chrome/browser/ui/cocoa/download/download_danger_prompt_impl.cc
[rename] https://crrev.com/64351f1c48aebf9167b6b1a0cfcec6a1779fbffa/chrome/browser/ui/proximity_auth/proximity_auth_error_bubble_stub.cc
[delete] https://crrev.com/5e83219cd5e9db35e916cda86cf0e085316821cb/chrome/browser/ui/views/frame/immersive_mode_controller_factory_mac.cc
[modify] https://crrev.com/64351f1c48aebf9167b6b1a0cfcec6a1779fbffa/chrome/browser/web_applications/web_app_mac.mm
[modify] https://crrev.com/64351f1c48aebf9167b6b1a0cfcec6a1779fbffa/chrome/chrome_browser_ui.gypi

Comment 4 by tapted@chromium.org, May 17 2016

Cc: tapted@chromium.org
Owner: ----
Status: Available (was: Started)
That gets `chrome` compiling. Other targets might need some TLC, but popping this off my runqueue for now.

And yeah, re #c2, go/macviewsbuilder might indeed be a good sandbox for switching to GN.. I guess there isn't a ton of gyp-specific stuff for mac_views_browser, but that might change if other views platforms turn down gyp before we do on mac.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/63a7fcd115f12a5f955014ab805cae1a6934c432

commit 63a7fcd115f12a5f955014ab805cae1a6934c432
Author: rsesek <rsesek@chromium.org>
Date: Wed Jun 15 16:03:20 2016

[Mac/GN] Remove views_test_suite.cc from macviews_interactive_ui_tests sources.

It is already part of //ui/views:test_support, which is a dependency.

BUG= 619781 , 611324 
TBR=sky@chromium.org

Review-Url: https://codereview.chromium.org/2056313003
Cr-Commit-Position: refs/heads/master@{#399910}

[modify] https://crrev.com/63a7fcd115f12a5f955014ab805cae1a6934c432/ui/views/BUILD.gn

Blocking: 431177
Blocking: -431177
As suggested by laforge@ and a conversation w/ the monorail folks, I'm going to try tracking GN-Migration related issues by *just* using the Proj-GN-Migration label, and not using blocking/rollup bugs, so that we can use blocking for just tasks that truly need to be completed before other tasks can make progress.

Comment 8 by rsesek@chromium.org, Jul 27 2016

Status: Fixed (was: Available)
d3b350b39b0f2f79881778f6c026ad7dd6c12432  flipped the MacVIews 10.10 bot and it passes compile: https://build.chromium.org/p/chromium.fyi/builders/Chromium%20Mac%2010.10%20MacViews/builds/11916 

Sign in to add a comment