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

[Temp] Declare v8_use_external_startup_data in build_overrides/v8.gni #381

Merged
merged 1 commit into from
Aug 29, 2016

Conversation

rakuco
Copy link
Member

@rakuco rakuco commented Aug 29, 2016

This is the Chromium side of crosswalk-project/v8-crosswalk#166.

Kind of revert 0908bc7 ("[Backport] gn: Remove unnecessary v8 defaults")
in the sense that v8_use_external_startup_data is being declared in
build_overrides/v8.gni again, but it is now enclosed in a declare_args
block.

We cannot just get rid of it as intended in 0908bc7, as M52's V8 does
not contain gni/v8.gni and a lot of declarations are done in V8's
top-level BUILD.gn instead.

Once we move to M53, V8 will work as intended and we will only need to
properly backport https://codereview.chromium.org/2058033002 to
chromium-crosswalk.

This is the Chromium side of crosswalk-project/v8-crosswalk#166.

Kind of revert 0908bc7 ("[Backport] gn: Remove unnecessary v8 defaults")
in the sense that `v8_use_external_startup_data` is being declared in
build_overrides/v8.gni again, but it is now enclosed in a `declare_args`
block.

We cannot just get rid of it as intended in 0908bc7, as M52's V8 does
not contain `gni/v8.gni` and a lot of declarations are done in V8's
top-level `BUILD.gn` instead.

Once we move to M53, V8 will work as intended and we will only need to
properly backport https://codereview.chromium.org/2058033002 to
chromium-crosswalk.
@crosswalk-trybot
Copy link

crosswalk-trybot commented Aug 29, 2016

Testing patch series with rakuco/chromium-crosswalk@44aed2e as its head.

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

@rakuco
Copy link
Member Author

rakuco commented Aug 29, 2016

@heke123: FYI. This is part 2 of 2 of the fix for the problem you mentioned in #378.

@mrunalk and @msisov: as mentioned in the PR message, neither this change nor 0908bc7 are needed in M53. Instead, https://codereview.chromium.org/2058033002 needs to be backported.

@darktears
Copy link
Contributor

lgtm

@rakuco rakuco merged commit 0a4d8d4 into crosswalk-project:master Aug 29, 2016
@rakuco rakuco deleted the gn-external_snapshot-arg branch August 29, 2016 15:05
@mrunalk
Copy link
Contributor

mrunalk commented Sep 1, 2016

Thanks @rakuco. I have removed those 2 changes in next in latest update from master and backported the upstream commit you mentioned.

imreotto pushed a commit to tenta-browser/chromium-crosswalk that referenced this pull request Nov 2, 2017
When WebMediaPlayer is destroyed while remoting, the remoting UI should
be hidden.

Previous approach (https://chromium-review.googlesource.com/c/581950)
added the notification in RendererController's destructor, which
requires proper destruct order in WebMediaPlayerImpl, and potentially
cause crash. This CL reverted previous change and instead hide the
remoting UI directly in media element when media player is cleared.

[email protected]

(cherry picked from commit 8729914)

Bug: 761699
Change-Id: I10452f93785a8e5814b4cffe466846d55962fa1e
Reviewed-on: https://chromium-review.googlesource.com/600950
Commit-Queue: Yuri Wiitala <[email protected]>
Reviewed-by: Mounir Lamouri <[email protected]>
Reviewed-by: Yuri Wiitala <[email protected]>
Reviewed-by: Chrome Cunningham <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#501741}
Reviewed-on: https://chromium-review.googlesource.com/676756
Reviewed-by: Xiangjun Zhang <[email protected]>
Cr-Commit-Position: refs/branch-heads/3202@{crosswalk-project#381}
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
Development

Successfully merging this pull request may close these issues.

4 participants