New issue
Advanced search Search tips

Issue 799064 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

AFDO profile for Chrome causing issues in webrtc

Project Member Reported by agrieve@chromium.org, Jan 4 2018

Issue description

From: https://chromium-review.googlesource.com/c/chromium/src/+/843774

This causes an unwanted directory to appear in WebRTC checkout, which messes up our automated systems besides being annoying. Could we not have references to //chrome in //build (which is supposed to be mostly generic)?

$ tree chrome
chrome
└── android
    └── profiles
        └── chrome-profile-3309-text.prof -> ../../../.cipd/pkgs/19/_current/chrome-profile-3309-text.prof
$ git status
Untracked files:
  (use "git add <file>..." to include in what will be committed)
        chrome/


Looks like we need to create another .ensure file somewhere for Chrome-specific packages. Maybe //DEPS.ensure?
 
Cc: mbonadei@chromium.org
Thanks for looking into this.

Comment 2 by g...@chromium.org, Jan 4 2018

Thanks!

Because I accidentally turned AFDO on for worlds outside of chromium, we'll need to fix that first. https://chromium-review.googlesource.com/c/chromium/src/+/850817 attempts to do so.
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71360159a54c1b6e0ecde4102c45176cd0f4b5e6

commit 71360159a54c1b6e0ecde4102c45176cd0f4b5e6
Author: George Burgess IV <gbiv@chromium.org>
Date: Thu Jan 04 23:17:20 2018

Only apply Chromium profiles in Chromium builds

An unintended side-effect of CL:847902 is that it enabled AFDO on
arbitrary projects that aren't Chrome, the browser (e.g. webrtc_android). While
it may be interesting to see if those profiles are helpful at some point, turn
them off for now.

local chromium checkout; profile is still used.

Bug:  799064 
Test: Applied to webrtc_android; profile usage is gone. Applied to a
Change-Id: Iea56d9be774193cc86d8e6b930a3c93345cd8a97
Reviewed-on: https://chromium-review.googlesource.com/850817
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: George Burgess <gbiv@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527138}
[modify] https://crrev.com/71360159a54c1b6e0ecde4102c45176cd0f4b5e6/build/config/android/config.gni
[modify] https://crrev.com/71360159a54c1b6e0ecde4102c45176cd0f4b5e6/build/config/android/sdk.gni

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bfc93d720491020defa4573a0d4da4ac7918e700

commit bfc93d720491020defa4573a0d4da4ac7918e700
Author: George Burgess IV <gbiv@chromium.org>
Date: Wed Jan 10 04:11:07 2018

Only download Chromium profiles for Chromium

CL:843774 made us download these profiles for Chromium and subprojects
(e.g. webrtc). We don't (yet?) want to apply profiles to said
subprojects, so we shouldn't be pulling them in the first place.

Bug:  799064 
Test: AFDO build of apks/Chrome.apk (among others) succeeds
Change-Id: Id1d34e1cc442c4871191f47564d6b625cce36f42
Reviewed-on: https://chromium-review.googlesource.com/851068
Commit-Queue: George Burgess <gbiv@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528237}
[modify] https://crrev.com/bfc93d720491020defa4573a0d4da4ac7918e700/DEPS
[modify] https://crrev.com/bfc93d720491020defa4573a0d4da4ac7918e700/build/cipd/android/android.ensure
[modify] https://crrev.com/bfc93d720491020defa4573a0d4da4ac7918e700/build/cipd/cipd_wrapper.py
[modify] https://crrev.com/bfc93d720491020defa4573a0d4da4ac7918e700/chrome/android/OWNERS
[add] https://crrev.com/bfc93d720491020defa4573a0d4da4ac7918e700/chrome/android/android.ensure

Cc: -g...@chromium.org
Owner: g...@chromium.org

Comment 6 by g...@chromium.org, Jan 12 2018

Status: Fixed (was: Assigned)
r528237 should've fixed this. Please let me know if I missed anything.

Thanks!

Sign in to add a comment