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

Issue 12045031: Disabling automation on Android. (Closed)

Created:
7 years, 11 months ago by Jay Civelli
Modified:
7 years, 11 months ago
CC:
chromium-reviews, darin-cc_chromium.org
Visibility:
Public.

Description

Disabling automation on Android. Not compiling the automation code on Android where it is not used. BUG=None TEST=All targets should still build on Android. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=178375

Patch Set 1 #

Total comments: 2

Patch Set 2 : Addressed John's remarks. #

Total comments: 2

Patch Set 3 : Addressed last suggestion from John #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -2 lines) Patch
M build/common.gypi View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/chrome_common.gypi View 1 2 chunks +5 lines, -1 line 0 comments Download
M chrome/chrome_renderer.gypi View 1 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/common/all_messages.h View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/renderer/chrome_content_renderer_client.cc View 1 3 chunks +6 lines, -1 line 0 comments Download

Messages

Total messages: 12 (0 generated)
Jay Civelli
7 years, 11 months ago (2013-01-22 21:23:35 UTC) #1
jam
can you actually reuse the enable_automation gypi variable? that'll also exclude chrome/browser/automation https://codereview.chromium.org/12045031/diff/1/chrome/chrome_renderer.gypi File chrome/chrome_renderer.gypi ...
7 years, 11 months ago (2013-01-22 21:27:08 UTC) #2
Jay Civelli
https://codereview.chromium.org/12045031/diff/1/chrome/chrome_renderer.gypi File chrome/chrome_renderer.gypi (right): https://codereview.chromium.org/12045031/diff/1/chrome/chrome_renderer.gypi#newcode330 chrome/chrome_renderer.gypi:330: 'renderer/automation/automation_renderer_helper.cc', On 2013/01/22 21:27:08, John Abd-El-Malek wrote: > can ...
7 years, 11 months ago (2013-01-22 21:55:25 UTC) #3
jam
lgtm, thanks https://codereview.chromium.org/12045031/diff/1005/chrome/common/all_messages.h File chrome/common/all_messages.h (right): https://codereview.chromium.org/12045031/diff/1005/chrome/common/all_messages.h#newcode18 chrome/common/all_messages.h:18: #include "chrome/common/automation_messages.h" nit: use ENABLE_AUTOMATION here
7 years, 11 months ago (2013-01-22 22:01:24 UTC) #4
cpu_(ooo_6.6-7.5)
lgtm x 2
7 years, 11 months ago (2013-01-22 22:09:14 UTC) #5
Jay Civelli
https://codereview.chromium.org/12045031/diff/1005/chrome/common/all_messages.h File chrome/common/all_messages.h (right): https://codereview.chromium.org/12045031/diff/1005/chrome/common/all_messages.h#newcode18 chrome/common/all_messages.h:18: #include "chrome/common/automation_messages.h" On 2013/01/22 22:01:24, John Abd-El-Malek wrote: > ...
7 years, 11 months ago (2013-01-22 22:50:05 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jcivelli@chromium.org/12045031/1006
7 years, 11 months ago (2013-01-23 02:30:17 UTC) #7
commit-bot: I haz the power
Presubmit check for 12045031-1006 failed and returned exit status 1. Running presubmit commit checks ...
7 years, 11 months ago (2013-01-23 02:30:20 UTC) #8
Jay Civelli
Justin, Could you look at the chrome/common changes? (I need an owner LGTM)
7 years, 11 months ago (2013-01-23 16:46:30 UTC) #9
jschuh
lgtm
7 years, 11 months ago (2013-01-23 18:59:35 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jcivelli@chromium.org/12045031/1006
7 years, 11 months ago (2013-01-23 19:05:57 UTC) #11
commit-bot: I haz the power
7 years, 11 months ago (2013-01-23 20:54:53 UTC) #12
Message was sent while issue was closed.
Change committed as 178375

Powered by Google App Engine
This is Rietveld 408576698