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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 118803004: Add option to compile out DCHECKs in android webview (Closed)

Created:
7 years ago by boliu
Modified:
6 years, 11 months ago
Reviewers:
Nico, Torne
CC:
chromium-reviews, craigdh+watch_chromium.org, bulach+watch_chromium.org, yfriedman+watch_chromium.org, ilevy-cc_chromium.org, klundberg+watch_chromium.org, frankf+watch_chromium.org
Visibility:
Public.

Description

Add option to compile out DCHECKs in android webview To support performance testing since DCHECKs can have a measurable impact on performance. BUG= NOTRY=true Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=242778

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -0 lines) Patch
M build/android/envsetup_functions.sh View 1 chunk +4 lines, -0 lines 2 comments Download

Messages

Total messages: 9 (0 generated)
boliu
https://codereview.chromium.org/118803004/diff/1/build/android/envsetup_functions.sh File build/android/envsetup_functions.sh (right): https://codereview.chromium.org/118803004/diff/1/build/android/envsetup_functions.sh#newcode324 build/android/envsetup_functions.sh:324: if [[ -n "$CHROME_ANDROID_WEBVIEW_OFFICIAL_BUILD" ]]; then I tried just ...
7 years ago (2013-12-19 19:45:23 UTC) #1
boliu
Err...hold this. Broke tracing.
7 years ago (2013-12-19 20:19:13 UTC) #2
boliu
Never mind. Tracing is a separate problem.
7 years ago (2013-12-19 22:05:32 UTC) #3
Torne
lgtm
6 years, 11 months ago (2014-01-02 11:17:09 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/boliu@chromium.org/118803004/1
6 years, 11 months ago (2014-01-02 17:01:36 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/boliu@chromium.org/118803004/1
6 years, 11 months ago (2014-01-02 18:53:31 UTC) #6
commit-bot: I haz the power
Change committed as 242778
6 years, 11 months ago (2014-01-02 18:54:30 UTC) #7
Nico
https://codereview.chromium.org/118803004/diff/1/build/android/envsetup_functions.sh File build/android/envsetup_functions.sh (right): https://codereview.chromium.org/118803004/diff/1/build/android/envsetup_functions.sh#newcode324 build/android/envsetup_functions.sh:324: if [[ -n "$CHROME_ANDROID_WEBVIEW_OFFICIAL_BUILD" ]]; then On 2013/12/19 19:45:24, ...
6 years, 11 months ago (2014-01-13 19:55:58 UTC) #8
boliu
6 years, 11 months ago (2014-01-13 21:55:14 UTC) #9
Message was sent while issue was closed. 
On 2014/01/13 19:55:58, Nico wrote:
>
https://codereview.chromium.org/118803004/diff/1/build/android/envsetup_funct...
> File build/android/envsetup_functions.sh (right):
> 
>
https://codereview.chromium.org/118803004/diff/1/build/android/envsetup_funct...
> build/android/envsetup_functions.sh:324: if [[ -n
> "$CHROME_ANDROID_WEBVIEW_OFFICIAL_BUILD" ]]; then
> On 2013/12/19 19:45:24, boliu wrote:
> > I tried just re-using CHROME_ANDROID_OFFICIAL_BUILD, but that also added
> > -DGOOGLE_CHROME_BUILD. Didn't go through all the instances, but looks like
> most
> > of them are harmless.
> 
> What's the problem with this? All the desktop chromes use official builds on
the
> perf waterfall (see crbug.com/271252). Why is this not possible for webview?

No one has tried it, so don't know if there are any problems. Maybe it just
works. Filed crbug.com/334021

Powered by Google App Engine
This is Rietveld 408576698