New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 805754 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

chrome::ResultCode is out of sync with enums.xml

Project Member Reported by thestig@chromium.org, Jan 25 2018

Issue description

Right now RESULT_CODE_INVALID_SANDBOX_STATE has the value of 31, but enums.xml thinks it should be 30. As a result, bugs about result codes, like  bug 786881  and bug 804463, may be looking at the wrong codes.

chrome::ResultCode enum values are offset from content::ResultCode enum values. So when content::ResultCode changes, so does chrome::ResultCode, and the values become out of sync with <enum name="CrashExitCodes"> in tools/metrics/histograms/enums.xml.

This off by 1 error happened in r444255, so this bug affects M57+.

Some contributing factors to this bug:
- There's no comment in the C++ code that mentions the enum value and the need to keep values in sync.
- There are no other enforcement mechanisms.
- content/public/common/result_codes.h and chrome/common/chrome_result_codes.h do not have file-specific OWNERS.
 
Owner: thestig@chromium.org
Status: Started (was: Untriaged)
Looks like I get to fix this.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 27 2018

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

commit 798fc420cf609933e469dc69e3828424efc28fd8
Author: Lei Zhang <thestig@chromium.org>
Date: Sat Jan 27 08:17:12 2018

Add check to keep chrome::ResultCode in sync with enums.xml.

Otherwise, it is too easy to accidentally change the enum value and make
it go out of sync. In fact, that is the current state. Update enums.xml
so the values line up again.

Also add a comment to remind readers to keep the enum values constant,
and mark a chrome::ResultCode enum as unused.

BUG= 805754 

Change-Id: I66efe023d5b8f5caa9f279641aea0f52a667ab0f
Reviewed-on: https://chromium-review.googlesource.com/885090
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532203}
[modify] https://crrev.com/798fc420cf609933e469dc69e3828424efc28fd8/chrome/common/chrome_result_codes.h
[modify] https://crrev.com/798fc420cf609933e469dc69e3828424efc28fd8/content/public/common/result_codes.h
[modify] https://crrev.com/798fc420cf609933e469dc69e3828424efc28fd8/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/798fc420cf609933e469dc69e3828424efc28fd8/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Started)

Sign in to add a comment