ChromeSSLHostStateDelegate cleanup |
|||||||
Issue descriptionThis class has a number of style problems and extra code that could use cleanup: 1.) SSLHostStateDelegate overrides aren't grouped properly in the .h 2.) Function definitions don't match declaration order 3.) The class contains experiment logic that can be cleaned up; the Finch experiment has launched so a bunch of code can be deleted for e.g. clearing exceptions at the end of a session.
,
Jun 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ba18a4b63d68610518fa8f27b7d2482431ab7740 commit ba18a4b63d68610518fa8f27b7d2482431ab7740 Author: Amos Lim <eui-sang.lim@samsung.com> Date: Fri Jun 08 05:53:24 2018 Cleanup ChromeSSLHostStateDelegate Group overrides and match function definitions and declartions order. Change QueryPolicy to take an int argument instead of a net::CertStatus. Bug: 840045 Change-Id: I6aa960015ffc2f81ab73957c2e14451143e6f8df Reviewed-on: https://chromium-review.googlesource.com/1065955 Reviewed-by: Richard Coles <torne@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Emily Stark <estark@chromium.org> Commit-Queue: Amos Lim <eui-sang.lim@samsung.com> Cr-Commit-Position: refs/heads/master@{#565561} [modify] https://crrev.com/ba18a4b63d68610518fa8f27b7d2482431ab7740/android_webview/browser/aw_ssl_host_state_delegate.cc [modify] https://crrev.com/ba18a4b63d68610518fa8f27b7d2482431ab7740/android_webview/browser/aw_ssl_host_state_delegate.h [modify] https://crrev.com/ba18a4b63d68610518fa8f27b7d2482431ab7740/chrome/browser/ssl/chrome_ssl_host_state_delegate.cc [modify] https://crrev.com/ba18a4b63d68610518fa8f27b7d2482431ab7740/chrome/browser/ssl/chrome_ssl_host_state_delegate.h [modify] https://crrev.com/ba18a4b63d68610518fa8f27b7d2482431ab7740/chrome/browser/ssl/chrome_ssl_host_state_delegate_test.cc [modify] https://crrev.com/ba18a4b63d68610518fa8f27b7d2482431ab7740/content/public/browser/ssl_host_state_delegate.h [modify] https://crrev.com/ba18a4b63d68610518fa8f27b7d2482431ab7740/content/test/mock_ssl_host_state_delegate.cc [modify] https://crrev.com/ba18a4b63d68610518fa8f27b7d2482431ab7740/content/test/mock_ssl_host_state_delegate.h
,
Jul 16
,
Jul 16
,
Jul 16
This is not fixed yet, #3 still needs to happen (clean up experiment logic).
,
Jul 18
,
Jul 18
,
Jul 18
livvielin@, fyi that when you add yourself as an owner you need to manually change the status to 'Assigned.' i don't know why it doesn't update itself automatically 🤷🏽♀️ i went ahead and changed this one for you
,
Jul 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9890773ec8b76730e477d6ad34a6cf6b1e3f381e commit 9890773ec8b76730e477d6ad34a6cf6b1e3f381e Author: Livvie Lin <livvielin@chromium.org> Date: Thu Jul 19 05:00:32 2018 Clean up experiment logic in ChromeSSLHostStateDelegate. This deletes code that references kRevertCertificateErrorDecisionsFieldTrialName, the experiment for clearing certificate exception decisions at the end of a session. This includes deleting all code for FORGET_SSL_EXCEPTION_DECISIONS_AT_SESSION_END since we decided to launch REMEMBER_SSL_EXCEPTION_DECISIONS_FOR_DELTA. Bug: 840045 Change-Id: I6ea2606a64c907e52215e20dc820b481ef28c114 Reviewed-on: https://chromium-review.googlesource.com/1142567 Commit-Queue: Livvie Lin <livvielin@chromium.org> Reviewed-by: Adrienne Porter Felt <felt@chromium.org> Cr-Commit-Position: refs/heads/master@{#576374} [modify] https://crrev.com/9890773ec8b76730e477d6ad34a6cf6b1e3f381e/chrome/browser/ssl/chrome_ssl_host_state_delegate.cc [modify] https://crrev.com/9890773ec8b76730e477d6ad34a6cf6b1e3f381e/chrome/browser/ssl/chrome_ssl_host_state_delegate.h [modify] https://crrev.com/9890773ec8b76730e477d6ad34a6cf6b1e3f381e/chrome/browser/ssl/chrome_ssl_host_state_delegate_test.cc
,
Jul 19
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by est...@chromium.org
, May 8 2018