|
|
Chromium Code Reviews|
Created:
9 years ago by palmer Modified:
9 years ago CC:
chromium-reviews, tmccoy, jeffreyc, Peter Kasting Base URL:
svn://svn.chromium.org/chrome/trunk/src/ Visibility:
Public. |
DescriptionRemove the wording that suggests it is possible to proceed after fatal SSL error.
But in cases where the error is not fatal and is overridable, add a string
saying you should not proceed. Now, all the messages that say "You should
not proceed..." use the same wording. This makes our message more clear.
Fix a "gcl lint" problem (DCHECK --> DCHECK_LT) while I'm here.
BUG=106254
TEST=compiles
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114744
Patch Set 1 #Patch Set 2 : '' #Patch Set 3 : '' #
Total comments: 2
Patch Set 4 : '' #Patch Set 5 : '' #
Total comments: 1
Patch Set 6 : '' #Patch Set 7 : '' #
Total comments: 12
Patch Set 8 : '' #
Messages
Total messages: 22 (0 generated)
Travis and Jeffrey: Any idea who is a good reviewer? There is no chrome/app/OWNERS or chrome/OWNERS.
Adding some SSL-related people in case they can review or know anyone who can.
Should we consider adding a sentence that simply says something like, "Unfortunately, you cannot proceed" and/or "Contact your network administrator", just to make it super clear? (sorry, don't know who would be the appropriate reviewer) On Wed, Dec 7, 2011 at 12:50 PM, <palmer@chromium.org> wrote: > Adding some SSL-related people in case they can review or know anyone who > can. > > http://codereview.chromium.**org/8856010/<http://codereview.chromium.org/8856... >
rsleevi pointed out that the error is not always fatal. It is only fatal for HSTS strict/pinned domains. So now I set the "you should not proceed..." text in the "extra info" section — same text as before, new string to contain it — when the error is non-fatal. Mind doing a little review, rsleevi? :)
Review comments on Patch Set 3: http://codereview.chromium.org/8856010/diff/9001/chrome/browser/ssl/ssl_block... File chrome/browser/ssl/ssl_blocking_page.cc (right): http://codereview.chromium.org/8856010/diff/9001/chrome/browser/ssl/ssl_block... chrome/browser/ssl/ssl_blocking_page.cc:97: l10n_util::GetStringUTF16(IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED)); Many of the certificate error messages have some sort of "you should not proceed" sentence at the end. The IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED message is specific to the CERT_STATUS_AUTHORITY_INVALID error. Ideally we should check that the certificate error is CERT_STATUS_AUTHORITY_INVALID right here. Can we ever get here for a certificate error other than CERT_STATUS_AUTHORITY_INVALID? For example, if there is a command-line option that forces all certificate errors to be fatal, will we end up here for an expired certificate? If so, then we will end up with two "you should not proceed" sentences.
http://codereview.chromium.org/8856010/diff/9001/chrome/browser/ssl/ssl_block... File chrome/browser/ssl/ssl_blocking_page.cc (right): http://codereview.chromium.org/8856010/diff/9001/chrome/browser/ssl/ssl_block... chrome/browser/ssl/ssl_blocking_page.cc:97: l10n_util::GetStringUTF16(IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED)); On 2011/12/08 00:35:37, wtc wrote: > > Many of the certificate error messages have some sort of > "you should not proceed" sentence at the end. The > IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED message is specific > to the CERT_STATUS_AUTHORITY_INVALID error. Ideally we should > check that the certificate error is CERT_STATUS_AUTHORITY_INVALID > right here. > > Can we ever get here for a certificate error other than > CERT_STATUS_AUTHORITY_INVALID? For example, if there is > a command-line option that forces all certificate errors > to be fatal, will we end up here for an expired certificate? > If so, then we will end up with two "you should not proceed" > sentences. Should this logic be pushed into SSLErrorInfo::CreateError? My concern with this approach (appending it to the bottom of |extra_info|) is that it may cause a serious warning to appear as supplemental info. Also, I can't recall, but doesn't |extra_info| show as a collapsible node (default collapsed), meaning that users wouldn't see this warning? It would seem that, to maintain parity with the current positioning of the warning, it should be moved closer to headline/description (lines 77/78). Perhaps one of the UI leads would have a better opinion / know if this is actually a concern. Also, like wtc said, a number of error messages have the proceed/don't proceed, and I believe that many of them are reasonable/possible to encounter with HSTS Strict/pinned certs. This CL may be fine for just that error, but the bug may not be resolved until a more generic approach is used / the remaining strings are updated. In particular, see IDS_CERT_ERROR_COMMON_NAME_INVALID_EXTRA_INFO_2, IDS_CERT_ERROR_EXPIRED_DETAILS, and IDS_CERT_ERROR_NOT_YET_VALID_DETAILS, which seem the most likely errors to be encountered with pinned/HSTS. Either for this as a follow-up, you may also wish to double check the text about proceeding with IDS_CERT_ERROR_CONTAINS_ERRORS_DETAILS - I don't believe you can skip past it.
> Many of the certificate error messages have some sort of
> "you should not proceed" sentence at the end. The
> IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED message is specific
> to the CERT_STATUS_AUTHORITY_INVALID error.
Actually, that is not true — SSLErrorInfo::CreateError uses the string
IDS_CERT_ERROR_AUTHORITY_INVALID_DETAILS in
case CERT_COMMON_NAME_INVALID:
case CERT_AUTHORITY_INVALID:
(see chrome/browser/ssl/ssl_error_info.cc)
IDS_CERT_ERROR_AUTHORITY_INVALID_DETAILS formerly had the text that is now in
IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED. So this CL does not change the error
cases for which that text is included, only where it appears (in the extra info
bullet points) and when (non-fatal cases only). If we want to also change which
error cases we show it for, that is ok but I like to keep my CLs minimal (haha,
no really) and directly addressing the bug at hand. The bug at hand is,
IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED included in cases where you can't
proceed anyway.
I think it is correct to show the IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED text
in case CERT_COMMON_NAME_INVALID, so I don't think we should (create a new
bug+CL | update this CL) to not show it in that case.
> Ideally we should
> check that the certificate error is CERT_STATUS_AUTHORITY_INVALID
> right here.
I think SSLErrorInfo::CreateError has it right. See next reply to rsleevi about
information passing between caller and callee, as well.
> Can we ever get here for a certificate error other than
> CERT_STATUS_AUTHORITY_INVALID? For example, if there is
> a command-line option that forces all certificate errors
> to be fatal, will we end up here for an expired certificate?
I don't know.
> If so, then we will end up with two "you should not proceed"
> sentences.
How? I am pretty sure these are the only cases where that text appears. In any
case, what really matters is that there is no "Proceed Anyway" button — that's
where the rubber hits the road. And this CL makes it so that
IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED does not appear for fatal errors. If
the (hypothetical?) command line switch affects the overridable_ boolean
ssl_blocking_page.cc correctly, then this CL would also result in correct error
text (i.e. IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED not included).
> Should this logic be pushed into SSLErrorInfo::CreateError?
You could add a boolean to SLErrorInfo::CreateError's parameter list (possibly
with a default=false) telling it whether or not to include
IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED. However, since that message is only
relevant for two out of many error cases, it seemed like an ugly intrusion upon
the API. Therefore I decided to add the string in the caller.
> My concern with this approach (appending it to the bottom of |extra_info|) is
> that it may cause a serious warning to appear as supplemental info.
Yes, but I think the remaining text of IDS_CERT_ERROR_AUTHORITY_INVALID_DETAILS
is strong enough ("...an attacker may be trying to intercept your
communications." !). That is why the first version of this CL just got rid of
the IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED text.
The real security UI action is in the buttons that say [Back to Safety] and
[Proceed Anyway], and in the fact that the whole screen is red and scary.
Noodling around with the text as we are doing here is quite tertiary — I am
pretty sure very few people read any of this.
I could be convinced either way, and could also be convinced to make the extra
info bullet points always present and not hidden by default.
> Also, I
> can't recall, but doesn't |extra_info| show as a collapsible node (default
> collapsed), meaning that users wouldn't see this warning?
Right.
> It would seem that, to maintain parity with the current positioning of the
> warning, it should be moved closer to headline/description (lines 77/78).
> Perhaps one of the UI leads would have a better opinion / know if this is
> actually a concern.
Yes, we need some UI guidance in this whole area. But my goal with this CL was
to simply make the text not outright wrong as it currently is.
> Also, like wtc said, a number of error messages have the proceed/don't
proceed,
> and I believe that many of them are reasonable/possible to encounter with HSTS
> Strict/pinned certs. This CL may be fine for just that error, but the bug may
> not be resolved until a more generic approach is used / the remaining strings
> are updated.
>
> In particular, see IDS_CERT_ERROR_COMMON_NAME_INVALID_EXTRA_INFO_2,
> IDS_CERT_ERROR_EXPIRED_DETAILS, and IDS_CERT_ERROR_NOT_YET_VALID_DETAILS,
which
> seem the most likely errors to be encountered with pinned/HSTS.
>
> Either for this as a follow-up, you may also wish to double check the text
about
> proceeding with IDS_CERT_ERROR_CONTAINS_ERRORS_DETAILS - I don't believe you
can
> skip past it.
Yeah, I see what you mean. I kind of think we need a whole mini-project
dedicated to cleaning this UI and the messaging up. The UI and messaging are
very old, and various new security behaviors have been added recently. It's a
discussion for another day, but as intimated above, I believe the real action is
in the choices we present. Any words should be in the buttons, really. I can
imagine collapsing all of this into one error screen:
<body style="background-color: red">
<h1>Chrome cannot be sure this is really <strong><ph
name="DOMAIN">$1<ex>paypal.com</ex></ph></strong>!</h1>
if (overridable_) {
[ I know this is not really
<ph name="DOMAIN">
but Proceed Anyway ]
}
[ Take Me Back To Safety ]
if (!overridable_) {
[ Report This Problem to Google
and Take Me Back To Safety ]
}
for (problem in certificate_problems) {
<li>problem</li>
}
(Note that "report this problem to Google" is on my TODO list anyway for
fraudulent certificate chains)
On 2011/12/08 19:37:15, Chris P. wrote:
> > Many of the certificate error messages have some sort of
> > "you should not proceed" sentence at the end. The
> > IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED message is specific
> > to the CERT_STATUS_AUTHORITY_INVALID error.
>
> Actually, that is not true — SSLErrorInfo::CreateError uses the string
> IDS_CERT_ERROR_AUTHORITY_INVALID_DETAILS in
>
> case CERT_COMMON_NAME_INVALID:
> case CERT_AUTHORITY_INVALID:
>
> (see chrome/browser/ssl/ssl_error_info.cc)
In chrome\browser\ssl\ssl_error_info.cc, revision 113498,
case CERT_COMMON_NAME_INVALID uses
IDS_CERT_ERROR_COMMON_NAME_INVALID_DETAILS, which ends with
the sentence "You should not proceed."
To answer my own question, overridable_ can only be forced
to 'false' by the following code in url_request_http_job.cc:
} else if (IsCertificateError(result)) {
// We encountered an SSL certificate error. Ask our delegate to decide
// what we should do.
TransportSecurityState::DomainState domain_state;
const bool is_hsts_host =
context_->transport_security_state() &&
context_->transport_security_state()->GetDomainState(
&domain_state, request_info_.url.host(),
SSLConfigService::IsSNIAvailable(context_->ssl_config_service())) &&
domain_state.ShouldCertificateErrorsBeFatal();
NotifySSLCertificateError(transaction_->GetResponseInfo()->ssl_info,
is_hsts_host);
So I believe we only need to consider the HSTS case.
If a server is an HSTS host and its certificate has a
name mismatch error, I believe overridable_ will be forced
to false and the SSL blocking page won't have a "Proceed"
button. In that case, the IDS_CERT_ERROR_COMMON_NAME_INVALID_DETAILS
message will still end with the sentence "You should not
proceed.", and IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED
will be added to the "extra information" section.
Took rsleevi's suggestion and put the IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED
text back in the main description.
Spoke with wtc and we decided to apply the same logic about including/not
including IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED also in the case of
CERT_COMMON_NAME_INVALID. In doing so, I also harmonized the wording — now the
text is always IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED, instead of "You should
not proceed." for CERT_COMMON_NAME_INVALID and
IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED for CERT_AUTHORITY_INVALID.
There are other things about these strings that I would like to fix also; for
example, IDS_CERT_ERROR_NOT_YET_VALID_DETAILS suggests that the user check their
system clock, while IDS_CERT_ERROR_EXPIRED_DETAILS does not. A wrong clock could
account for either problem. Additionally, in generated_resources.grd, there are
three wordings ("You should not proceed.", "You should not proceed past this
point.", and "You absolutely should not proceed past this point."), but they are
in extra info blocks. Should I fix these problems (a) at all; and (b) in this CL
or in a new CL?
For all details blocks in all three *.grd files in this CL, I got rid of any
"You should not proceed..." wording, because
IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED will be appended to the text iff
overridable_.
Adding ifette as a reviewer at wtc's suggestion.
I do think it's valuable to fix the issues you brought up, but have no strong feelings either way on whether it's one CL or multiple CLs. Either way, we're past M17 branch point for strings, so it shouldn't matter about when in the M18 cycle it gets fixed. One note below about the approach taken - http://codereview.chromium.org/8856010/diff/10004/chrome/browser/ssl/ssl_bloc... File chrome/browser/ssl/ssl_blocking_page.cc (right): http://codereview.chromium.org/8856010/diff/10004/chrome/browser/ssl/ssl_bloc... chrome/browser/ssl/ssl_blocking_page.cc:94: error_info.details() + ASCIIToUTF16(" ") + I didn't think it was permissible to concatenate translated strings like this. You'd definitely want to check with someone more familiar with the internationalization/localization system.
> http://codereview.chromium.org/8856010/diff/10004/chrome/browser/ssl/ssl_bloc... > chrome/browser/ssl/ssl_blocking_page.cc:94: error_info.details() + > ASCIIToUTF16(" ") + > I didn't think it was permissible to concatenate translated strings like this. > You'd definitely want to check with someone more familiar with the > internationalization/localization system. Yes, I took pkasting's advice and just did it as a bunch of complete paragraphs. Then the best way to make that work was to pass an overridable boolean into CreateError, so I did that, too. Reviewers, anyone: LGTY?
Adding pkasting FYI and in case you want to review.
No LGTM from valid reviewers yet.
Thanks - I think this is a much cleaner approach, and it unifies all of them. A few extra strings remain, annotated below. They're all in EXTRA_INFO though, so perhaps they don't need to be changed. One other small note below, both otherwise LGTM. Just make sure to do a local test to see that things work. https://latest.foo.appspot.com/ for a non-overridable site, choose your favourite for an overridable site - https://webkit.org/ is always good http://codereview.chromium.org/8856010/diff/26001/chrome/app/chromium_strings... File chrome/app/chromium_strings.grd (right): http://codereview.chromium.org/8856010/diff/26001/chrome/app/chromium_strings... chrome/app/chromium_strings.grd:193: In this case, the address listed in the certificate does not match the address of the website your browser tried to go to. One possible reason for this is that your communications are being intercepted by an attacker who is presenting a certificate for a different website, which would cause a mismatch. Another possible reason is that the server is set up to return the same certificate for multiple websites, including the one you are attempting to visit, even though that certificate is not valid for all of those websites. Chromium can say for sure that you reached <strong><ph name="DOMAIN2">$1<ex>paypal.com</ex></ph></strong>, but cannot verify that that is the same site as <strong><ph name="DOMAIN">$2<ex>www.paypal.com</ex></ph></strong> which you intended to reach. If you proceed, Chromium will not check for any further name mismatches. In general, it is best not to proceed past this point. What about this instance? http://codereview.chromium.org/8856010/diff/26001/chrome/app/generated_resour... File chrome/app/generated_resources.grd (right): http://codereview.chromium.org/8856010/diff/26001/chrome/app/generated_resour... chrome/app/generated_resources.grd:2743: For a certificate which has not expired, the issuer of that certificate is responsible for maintaining something called a "revocation list". If a certificate is ever compromised, the issuer can revoke it by adding it to the revocation list, and then this certificate will no longer be trusted by your browser. Revocation status is not required to be maintained for expired certificates, so while this certificate used to be valid for the website you're visiting, at this point it is not possible to determine whether the certificate was compromised and subsequently revoked, or whether it remains secure. As such it is impossible to tell whether you're communicating with the legitimate website, or whether the certificate was compromised and is now in the possession of an attacker with whom you are communicating. You should not proceed past this point. And this? http://codereview.chromium.org/8856010/diff/26001/chrome/app/generated_resour... chrome/app/generated_resources.grd:2809: In this case, the certificate presented to your browser has been revoked by its issuer. This usually means that the integrity of this certificate has been compromised, and that the certificate should not be trusted. You absolutely should not proceed past this point. And this http://codereview.chromium.org/8856010/diff/26001/chrome/app/generated_resour... chrome/app/generated_resources.grd:2835: In this case, the server certificate or an intermediate CA certificate presented to your browser is signed using a weak signature algorithm such as RSA-MD2. Recent research by computer scientists showed the signature algorithm is weaker than previously believed, and the signature algorithm is rarely used by trustworthy websites today. This certificate could have been forged. You should not proceed past this point. Here? http://codereview.chromium.org/8856010/diff/26001/chrome/app/generated_resour... chrome/app/generated_resources.grd:7662: <message name="IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED" desc="Message advising the user not to proceed."> From the most recent e-mails re: i18n/l10n, I don't believe the translators see the ID Perhaps then you should clarify "Message advising the user not to proceed past the SSL blocking page" ? http://codereview.chromium.org/8856010/diff/26001/chrome/app/google_chrome_st... File chrome/app/google_chrome_strings.grd (right): http://codereview.chromium.org/8856010/diff/26001/chrome/app/google_chrome_st... chrome/app/google_chrome_strings.grd:167: In this case, the address listed in the certificate does not match the address of the website your browser tried to go to. One possible reason for this is that your communications are being intercepted by an attacker who is presenting a certificate for a different website, which would cause a mismatch. Another possible reason is that the server is set up to return the same certificate for multiple websites, including the one you are attempting to visit, even though that certificate is not valid for all of those websites. Google Chrome can say for sure that you reached <strong><ph name="DOMAIN2">$1<ex>paypal.com</ex></ph></strong>, but cannot verify that that is the same site as <strong><ph name="DOMAIN">$2<ex>www.paypal.com</ex></ph></strong> which you intended to reach. If you proceed, Chrome will not check for any further name mismatches. In general, it is best not to proceed past this point. Here
http://codereview.chromium.org/8856010/diff/26001/chrome/app/chromium_strings... File chrome/app/chromium_strings.grd (right): http://codereview.chromium.org/8856010/diff/26001/chrome/app/chromium_strings... chrome/app/chromium_strings.grd:193: In this case, the address listed in the certificate does not match the address of the website your browser tried to go to. One possible reason for this is that your communications are being intercepted by an attacker who is presenting a certificate for a different website, which would cause a mismatch. Another possible reason is that the server is set up to return the same certificate for multiple websites, including the one you are attempting to visit, even though that certificate is not valid for all of those websites. Chromium can say for sure that you reached <strong><ph name="DOMAIN2">$1<ex>paypal.com</ex></ph></strong>, but cannot verify that that is the same site as <strong><ph name="DOMAIN">$2<ex>www.paypal.com</ex></ph></strong> which you intended to reach. If you proceed, Chromium will not check for any further name mismatches. In general, it is best not to proceed past this point. On 2011/12/15 01:50:04, Ryan Sleevi wrote: > What about this instance? Done. http://codereview.chromium.org/8856010/diff/26001/chrome/app/generated_resour... File chrome/app/generated_resources.grd (right): http://codereview.chromium.org/8856010/diff/26001/chrome/app/generated_resour... chrome/app/generated_resources.grd:2743: For a certificate which has not expired, the issuer of that certificate is responsible for maintaining something called a "revocation list". If a certificate is ever compromised, the issuer can revoke it by adding it to the revocation list, and then this certificate will no longer be trusted by your browser. Revocation status is not required to be maintained for expired certificates, so while this certificate used to be valid for the website you're visiting, at this point it is not possible to determine whether the certificate was compromised and subsequently revoked, or whether it remains secure. As such it is impossible to tell whether you're communicating with the legitimate website, or whether the certificate was compromised and is now in the possession of an attacker with whom you are communicating. You should not proceed past this point. On 2011/12/15 01:50:04, Ryan Sleevi wrote: > And this? Done. http://codereview.chromium.org/8856010/diff/26001/chrome/app/generated_resour... chrome/app/generated_resources.grd:2809: In this case, the certificate presented to your browser has been revoked by its issuer. This usually means that the integrity of this certificate has been compromised, and that the certificate should not be trusted. You absolutely should not proceed past this point. On 2011/12/15 01:50:04, Ryan Sleevi wrote: > And this Done. http://codereview.chromium.org/8856010/diff/26001/chrome/app/generated_resour... chrome/app/generated_resources.grd:2835: In this case, the server certificate or an intermediate CA certificate presented to your browser is signed using a weak signature algorithm such as RSA-MD2. Recent research by computer scientists showed the signature algorithm is weaker than previously believed, and the signature algorithm is rarely used by trustworthy websites today. This certificate could have been forged. You should not proceed past this point. On 2011/12/15 01:50:04, Ryan Sleevi wrote: > Here? Done. http://codereview.chromium.org/8856010/diff/26001/chrome/app/generated_resour... chrome/app/generated_resources.grd:7662: <message name="IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED" desc="Message advising the user not to proceed."> On 2011/12/15 01:50:04, Ryan Sleevi wrote: > From the most recent e-mails re: i18n/l10n, I don't believe the translators see > the ID > > Perhaps then you should clarify "Message advising the user not to proceed past > the SSL blocking page" ? Done. http://codereview.chromium.org/8856010/diff/26001/chrome/app/google_chrome_st... File chrome/app/google_chrome_strings.grd (right): http://codereview.chromium.org/8856010/diff/26001/chrome/app/google_chrome_st... chrome/app/google_chrome_strings.grd:167: In this case, the address listed in the certificate does not match the address of the website your browser tried to go to. One possible reason for this is that your communications are being intercepted by an attacker who is presenting a certificate for a different website, which would cause a mismatch. Another possible reason is that the server is set up to return the same certificate for multiple websites, including the one you are attempting to visit, even though that certificate is not valid for all of those websites. Google Chrome can say for sure that you reached <strong><ph name="DOMAIN2">$1<ex>paypal.com</ex></ph></strong>, but cannot verify that that is the same site as <strong><ph name="DOMAIN">$2<ex>www.paypal.com</ex></ph></strong> which you intended to reach. If you proceed, Chrome will not check for any further name mismatches. In general, it is best not to proceed past this point. On 2011/12/15 01:50:04, Ryan Sleevi wrote: > Here Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/palmer@chromium.org/8856010/28004
Presubmit check for 8856010-28004 failed and returned exit status 1. Running presubmit commit checks ... ** Presubmit ERRORS ** Missing LGTM from an OWNER for: chrome/browser/resources/ssl_roadblock.html
lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/palmer@chromium.org/8856010/28004
Change committed as 114744 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
