Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Add GN build support for crosswalk-linux #375

Merged
merged 1 commit into from
Aug 26, 2016
Merged

Add GN build support for crosswalk-linux #375

merged 1 commit into from
Aug 26, 2016

Conversation

heke123
Copy link
Contributor

@heke123 heke123 commented Aug 17, 2016

As Chromium will deprecate GYP build system since M54, we need to enable
GN build support for Crosswalk as soon as possible.
Set xwalk as "extra_deps" to Chromium in xwalk/build/common.gni:
root_extra_deps = [ "//xwalk:xwalk_builder" ]

@crosswalk-trybot
Copy link

crosswalk-trybot commented Aug 17, 2016

Testing patch series with heke123/chromium-crosswalk@d9f293d0faea58215f930ac00af500b059928bcb as its head.

Bot Status
Content Shell Android-x86 [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/327)
Content Shell Linux [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/325)

@heke123
Copy link
Contributor Author

heke123 commented Aug 17, 2016

@rakuco

@rakuco
Copy link
Member

rakuco commented Aug 17, 2016

The changes look fine to me, but they need to be committed differently: the enable_webvr part should be a separate commit backported from upstream (use git cherry-pick, edit the commit message to mention it is a backport like this one and make sure it does not contain the "Cr-Commit-Position" line) and then you can add xwalk to the exec_script whitelist in another one (and explain why it is needed).

@rakuco
Copy link
Member

rakuco commented Aug 17, 2016

And there's nothing Linux-specific here, so I'd retitle the PR and not mention Linux in the commit where you change the exec_script whitelist.

@rakuco
Copy link
Member

rakuco commented Aug 17, 2016

One last thing: don't you also need to port our chromium-crosswalk changes to GN as well? The following commits touch gyp files:

  • c72323e ("[Extensions] Build glib message pump on Ozone by default")
  • 29782db ("Add flag to disable hrtf in webaudio")
  • 84686b5 ("[Windows] Support RealSense Cameras")
  • 63e1f41 ("[Temp] Do not enable generate_multidex_config by default on Android.")
  • 2b2cbc8 ("[Windows] Implementation of 'AudioDestinationNode.devicePosition' attribute")
  • ad2a7f0 ("[Temp] Make |enable_webvr==0| work.")

Most of those are not related to Linux so you don't necessarily have to port all of them here, but I thought it'd be good to bring them up in case you were not considering them.

@heke123
Copy link
Contributor Author

heke123 commented Aug 23, 2016

@rakuco Thanks very much for remind me this.
for this one:
2b2cbc8 ("[Windows] Implementation of 'AudioDestinationNode.devicePosition' attribute")
The title is for windows only, but seems the gyp modification has affects on all platforms, should I port it now?

And below 2 commits will be port into GN soon.
c72323e ("[Extensions] Build glib message pump on Ozone by default")
29782db ("Add flag to disable hrtf in webaudio")

@rakuco
Copy link
Member

rakuco commented Aug 23, 2016

The title is for windows only, but seems the gyp modification has affects on all platforms, should I port it now?

Yes, I think you'll need to port this one too (it should be a matter of adding a few lines to some BUILD.gn files).

@rakuco
Copy link
Member

rakuco commented Aug 24, 2016

63e1f41 ("[Temp] Do not enable generate_multidex_config by default on Android.")

There doesn't seem to be anything to do with this one: as far as I can see, each target needs to set enable_multidex = true for the multidex stuff to be enabled (i.e. it becomes opt-in instead of opt-out, which is what we want)

@rakuco
Copy link
Member

rakuco commented Aug 24, 2016

FYI, I'm handling the Android-specific changes in #377.

@heke123
Copy link
Contributor Author

heke123 commented Aug 26, 2016

@rakuco
2b2cbc8 ("[Windows] Implementation of 'AudioDestinationNode.devicePosition' attribute")
It contains the BUILD.gn. so we don't need do anything.

You intent to remove below 2 commits, I guess we don't need to port them either?
c72323e ("[Extensions] Build glib message pump on Ozone by default")
29782db ("Add flag to disable hrtf in webaudio")

@rakuco
Copy link
Member

rakuco commented Aug 26, 2016

2b2cbc8 ("[Windows] Implementation of 'AudioDestinationNode.devicePosition' attribute")
It contains the BUILD.gn. so we don't need do anything.

Excellent!

You intent to remove below 2 commits, I guess we don't need to port them either?
c72323e ("[Extensions] Build glib message pump on Ozone by default")
29782db ("Add flag to disable hrtf in webaudio")

Yep, correct.

@crosswalk-trybot
Copy link

crosswalk-trybot commented Aug 26, 2016

Testing patch series with heke123/chromium-crosswalk@ffc2b3214a333fda6e31f3a724bdd951ec265ba3 as its head.

Bot Status
Content Shell Android-x86 [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/332)
Content Shell Linux [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/330)

@crosswalk-trybot
Copy link

crosswalk-trybot commented Aug 26, 2016

Testing patch series with heke123/chromium-crosswalk@bde5610d5025d1ec94dee063eba78d8a6012d5e5 as its head.

Bot Status
Content Shell Android-x86 [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/333)
Content Shell Linux [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/331)

Add "//xwalk/build/version.gni" the whitelist to make sure in
src/xwalk it can execute "exec_script".
@rakuco
Copy link
Member

rakuco commented Aug 26, 2016

yes! victory at last 👍

@rakuco rakuco merged commit f3fb6c0 into crosswalk-project:master Aug 26, 2016
@heke123
Copy link
Contributor Author

heke123 commented Aug 26, 2016

Thanks so much for your help!

imreotto pushed a commit to tenta-browser/chromium-crosswalk that referenced this pull request Nov 2, 2017
When saving an icon in background, the sandbox may already
be locked. This result in saving failure and an empty icon
being used in extension.
This CL
1. Avoid to save empty icon
2. If the widget meets an empty icon at some point, tries
to show a default icon instead of a black one.

Bug: 762517
Change-Id: I66c248f00524900e2c240acb94bc1ed4b5654d5f
Reviewed-on: https://chromium-review.googlesource.com/663261
Commit-Queue: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Eric Noyau <noyau@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#501928}(cherry picked from commit 8c6dd42)
Reviewed-on: https://chromium-review.googlesource.com/677363
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{crosswalk-project#375}
Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants