Chromium Code Reviews

Issue 1460633005: Aura on Android: Remove unnecessary dependencies on ui/android. (Closed)

Created:
5 years, 1 month ago by Hadi
Modified:
5 years, 1 month ago
Reviewers:
mfomitchev, no sievers
CC:
chromium-reviews, darin-cc_chromium.org, jam
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Aura on Android:Remove unnecessary dependencies on ui/android. BUG=NONE Committed: https://crrev.com/d2ff47131f1a53a712adf860275a2f7320d9da9f Cr-Commit-Position: refs/heads/master@{#360835}

Patch Set 1 : Minimal change to fix the build. #

Patch Set 2 : Put ui_android inside if (!use_aura). #

Patch Set 3 : Use assert(!use_aura). #

Total comments: 3

Patch Set 4 : Revert to patch set 1. #

Patch Set 5 : Fix content/app/BUILD.gn on Android when use_aura is not defined explicitly. #

Total comments: 3

Patch Set 6 : address feedback from ps5. #

Unified diffs Side-by-side diffs Stats (+6 lines, -3 lines)
M content/app/BUILD.gn View 2 chunks +4 lines, -1 line 0 comments
M content/browser/BUILD.gn View 2 chunks +1 line, -1 line 0 comments
M content/test/BUILD.gn View 1 chunk +1 line, -1 line 0 comments

Messages

Total messages: 21 (7 generated)
Hadi
Patch set #2 puts ui_android inside "if (!use_aura)" so we can catch if something depends ...
5 years, 1 month ago (2015-11-19 22:20:38 UTC) #3
no sievers
https://codereview.chromium.org/1460633005/diff/40001/components/web_contents_delegate_android/BUILD.gn File components/web_contents_delegate_android/BUILD.gn (right): https://codereview.chromium.org/1460633005/diff/40001/components/web_contents_delegate_android/BUILD.gn#newcode33 components/web_contents_delegate_android/BUILD.gn:33: deps += [ "//ui/android" ] this should not be ...
5 years, 1 month ago (2015-11-19 22:23:10 UTC) #4
Hadi
https://codereview.chromium.org/1460633005/diff/40001/ui/android/BUILD.gn File ui/android/BUILD.gn (right): https://codereview.chromium.org/1460633005/diff/40001/ui/android/BUILD.gn#newcode11 ui/android/BUILD.gn:11: assert(!use_aura) I think this doesn't work, since we don't ...
5 years, 1 month ago (2015-11-19 22:24:16 UTC) #5
Hadi
On 2015/11/19 22:23:10, sievers wrote: > https://codereview.chromium.org/1460633005/diff/40001/components/web_contents_delegate_android/BUILD.gn > File components/web_contents_delegate_android/BUILD.gn (right): > > https://codereview.chromium.org/1460633005/diff/40001/components/web_contents_delegate_android/BUILD.gn#newcode33 > ...
5 years, 1 month ago (2015-11-19 22:31:11 UTC) #6
Hadi
https://codereview.chromium.org/1460633005/diff/40001/components/web_contents_delegate_android/BUILD.gn File components/web_contents_delegate_android/BUILD.gn (right): https://codereview.chromium.org/1460633005/diff/40001/components/web_contents_delegate_android/BUILD.gn#newcode33 components/web_contents_delegate_android/BUILD.gn:33: deps += [ "//ui/android" ] On 2015/11/19 22:23:09, sievers ...
5 years, 1 month ago (2015-11-19 22:38:04 UTC) #7
mfomitchev
It's not that the GN file "evaluates" to ui_java or android. ui/android/BUILD.gn defines multiple "components"/"build ...
5 years, 1 month ago (2015-11-19 22:38:56 UTC) #8
no sievers
ps1 lgtm
5 years, 1 month ago (2015-11-19 22:55:20 UTC) #9
Hadi
https://codereview.chromium.org/1460633005/diff/80001/content/app/BUILD.gn File content/app/BUILD.gn (right): https://codereview.chromium.org/1460633005/diff/80001/content/app/BUILD.gn#newcode53 content/app/BUILD.gn:53: if (!defined(use_aura) || !use_aura) { mfomitchev@/sievers@ do you know ...
5 years, 1 month ago (2015-11-20 15:42:42 UTC) #11
mfomitchev
Can you also please update the title: it's not just content_unittests that is affected - ...
5 years, 1 month ago (2015-11-20 15:55:03 UTC) #12
Hadi
https://codereview.chromium.org/1460633005/diff/80001/content/app/BUILD.gn File content/app/BUILD.gn (right): https://codereview.chromium.org/1460633005/diff/80001/content/app/BUILD.gn#newcode53 content/app/BUILD.gn:53: if (!defined(use_aura) || !use_aura) { On 2015/11/20 15:55:03, mfomitchev ...
5 years, 1 month ago (2015-11-20 16:04:48 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1460633005/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1460633005/100001
5 years, 1 month ago (2015-11-20 16:29:02 UTC) #18
commit-bot: I haz the power
Committed patchset #6 (id:100001)
5 years, 1 month ago (2015-11-20 16:33:50 UTC) #19
commit-bot: I haz the power
Patchset 6 (id:??) landed as https://crrev.com/d2ff47131f1a53a712adf860275a2f7320d9da9f Cr-Commit-Position: refs/heads/master@{#360835}
5 years, 1 month ago (2015-11-20 16:35:19 UTC) #20
mfomitchev
4 years, 11 months ago (2016-01-27 21:37:49 UTC) #21
Message was sent while issue was closed.
A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1646693002/ by mfomitchev@chromium.org.

The reason for reverting is: Reverting, since Android Aura has been cancelled..

Powered by Google App Engine