ssl_browser_test uses -1 as a default failure value, but -1 means CMD_TEXT_NOT_FOUND. |
|
Issue description
In ssl_browser_tests.cc, a lot of tests do something like this?
int result = -1;
content::ExecuteScriptAndExtractInt(..., &result);
However, some tests *expect* -1 as a possible output, because -1 is security_interstitials::CMD_TEXT_NOT_FOUND.
We should probably change all those initializations to
int result = CMD_ERROR;
We may also want to reorder the enum (note: both C++ and JS) so that -1 is ERROR, since -1 is a common error value in other programming situations.
[1] https://cs.chromium.org/chromium/src/components/security_interstitials/core/controller_client.h?l=33&rcl=69ae10f91723897591ef1a3b465aa5d35011eb5e
,
Oct 14 2017
Submitted patch https://chromium-review.googlesource.com/c/chromium/src/+/720336 for review. Thanks !!!
,
Oct 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5be6c91896d063372a8be848f2410faed7c310cb commit 5be6c91896d063372a8be848f2410faed7c310cb Author: Kyoungdeok Kwon <kkd927@gmail.com> Date: Fri Oct 20 20:31:06 2017 ssl_browser_test uses -1 as a default failure value, but -1 means CMD_TEXT_NOT_FOUND. In ssl_browser_tests.cc, -1 is used as a default failure value, but -1 means security_interstitials::CMD_TEXT_NOT_FOUND. Probably change all those initializations to CMD_ERROR. R=meacer@chromium.org Bug: 771356 Change-Id: I2cfa73a31fd210bea45b8d238f1b18116413d4bf Reviewed-on: https://chromium-review.googlesource.com/720336 Reviewed-by: Lucas Garron <lgarron@chromium.org> Reviewed-by: Mustafa Emre Acer <meacer@chromium.org> Commit-Queue: Lucas Garron <lgarron@chromium.org> Cr-Commit-Position: refs/heads/master@{#510548} [modify] https://crrev.com/5be6c91896d063372a8be848f2410faed7c310cb/AUTHORS [modify] https://crrev.com/5be6c91896d063372a8be848f2410faed7c310cb/chrome/browser/ssl/ssl_browser_tests.cc [modify] https://crrev.com/5be6c91896d063372a8be848f2410faed7c310cb/components/security_interstitials/core/controller_client.h
,
Oct 23 2017
,
Oct 30 2017
> (note: both C++ and JS) For the record: JS didn't need any changes because SecurityInterstitialCommandId [1] doesn't contain negative test values. [1] https://cs.chromium.org/chromium/src/components/security_interstitials/core/common/resources/interstitial_common.js?l=10&rcl=6ca1d26588914930897a8370e925b8361fdbd4d6 |
|
►
Sign in to add a comment |
|
Comment 1 by mea...@chromium.org
, Oct 3 2017