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

Issue 500423004: Refactor/cleanup WebRTC-specific Andorid glue code. (Closed)

Created:
6 years, 3 months ago by kjellander_chromium
Modified:
6 years, 3 months ago
Reviewers:
navabi1
CC:
chromium-reviews, klundberg+watch_chromium.org, yfriedman+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Refactor/cleanup WebRTC-specific Andorid glue code. Remove a bunch of code that is already longer used or will become unused when https://review.webrtc.org/22149004/ and https://codereview.chromium.org/505153002/ are landed. Remove hardcoded paths to the WebRTC isolate files to make it easier for us to make changes to them, since we can pass the path to the .isolate file on test execution instead (keeping the configuration in the WebRTC buildbot recipe instead). Previously we had to roll our Chromium revision in WebRTC DEPS every time a change was made before it became used. Add support for a CHECKOUT_SOURCE_ROOT environment variable used to make it possible to override the hardcoded path traversal that is done to find the src/ directory (since it's different for WebRTC bots). I considered passing a flag into the test_runner.py scripts instead of using an environment variable, but my opinion is that it would pollute the code too much since constants.DIR_SOURCE_ROOT is used in 70 different places. BUG=webrtc:3741 TEST=local building and test execution using command lines like this: cd /path/to/webrtc/src export CHECKOUT_SOURCE_ROOT=`pwd` build/android/test_runner.py gtest -s tools_unittests --isolate-file-path=webrtc/tools/tools_unittests.isolate R=navabi@chromium.org Committed: https://chromium.googlesource.com/chromium/src/+/f0a439da1aba8e22b30ede87087418e806f7fa75

Patch Set 1 #

Total comments: 2

Patch Set 2 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -84 lines) Patch
M build/all.gyp View 1 1 chunk +0 lines, -18 lines 0 comments Download
M build/android/buildbot/bb_device_steps.py View 1 3 chunks +1 line, -11 lines 0 comments Download
M build/android/buildbot/bb_run_bot.py View 1 1 chunk +0 lines, -12 lines 0 comments Download
M build/android/pylib/constants.py View 1 1 chunk +3 lines, -2 lines 0 comments Download
M build/android/pylib/gtest/gtest_config.py View 1 chunk +0 lines, -17 lines 0 comments Download
M build/android/pylib/gtest/setup.py View 2 chunks +1 line, -24 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
kjellander_chromium
kjellander@chromium.org changed reviewers: + navabi@chromium.org
6 years, 3 months ago (2014-08-27 14:09:58 UTC) #1
kjellander_chromium
https://codereview.chromium.org/500423004/diff/1/build/all.gyp File build/all.gyp (left): https://codereview.chromium.org/500423004/diff/1/build/all.gyp#oldcode822 build/all.gyp:822: 'target_name': 'android_builder_chromium_webrtc', Our Chromium bots in http://build.chromium.org/p/chromium.webrtc/waterfall are still ...
6 years, 3 months ago (2014-08-27 14:09:59 UTC) #2
navabi1
lgtm as it only effect webrtc bots and Henrik knows those best. (Also adds an ...
6 years, 3 months ago (2014-08-27 18:58:01 UTC) #3
kjellander_chromium
Committed patchset #2 (id:20001) manually as f0a439d (presubmit successful).
6 years, 3 months ago (2014-09-01 11:10:40 UTC) #4
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 03:16:30 UTC) #5
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/f0a439da1aba8e22b30ede87087418e806f7fa75
Cr-Commit-Position: refs/heads/master@{#292861}

Powered by Google App Engine
This is Rietveld 408576698