Commit e25acef6 authored by Yi Su's avatar Yi Su Committed by Commit Bot

Move CRWWebController._certVerificationErrors into

CRWWKNavigationHandler.

This CL moves CRWWebController._certVerificationErrors into
CRWWKNavigationHandler since it's only used there.

Bug: 956511
Change-Id: Iaa7225a2181b720b0184f80243ff1f945a72e032
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1639390
Commit-Queue: Yi Su <mrsuyi@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#666204}
parent 5bb50560
...@@ -41,11 +41,6 @@ class WKBackForwardListItemHolder; ...@@ -41,11 +41,6 @@ class WKBackForwardListItemHolder;
- (web::UserInteractionState*)userInteractionStateForNavigationHandler: - (web::UserInteractionState*)userInteractionStateForNavigationHandler:
(CRWWKNavigationHandler*)navigationHandler; (CRWWKNavigationHandler*)navigationHandler;
// Returns associated certificate verification errors.
- (web::CertVerificationErrorsCacheType*)
certVerificationErrorsForNavigationHandler:
(CRWWKNavigationHandler*)navigationHandler;
// Returns associated certificate verificatio controller. // Returns associated certificate verificatio controller.
- (CRWCertVerificationController*) - (CRWCertVerificationController*)
certVerificationControllerForNavigationHandler: certVerificationControllerForNavigationHandler:
......
...@@ -53,6 +53,13 @@ using web::wk_navigation_util::kReferrerHeaderName; ...@@ -53,6 +53,13 @@ using web::wk_navigation_util::kReferrerHeaderName;
using web::wk_navigation_util::IsRestoreSessionUrl; using web::wk_navigation_util::IsRestoreSessionUrl;
using web::wk_navigation_util::IsWKInternalUrl; using web::wk_navigation_util::IsWKInternalUrl;
namespace {
// Maximum number of errors to store in cert verification errors cache.
// Cache holds errors only for pending navigations, so the actual number of
// stored errors is not expected to be high.
const web::CertVerificationErrorsCacheType::size_type kMaxCertErrorsCount = 100;
}
@interface CRWWKNavigationHandler () { @interface CRWWKNavigationHandler () {
// Used to poll for a SafeBrowsing warning being displayed. This is created in // Used to poll for a SafeBrowsing warning being displayed. This is created in
// |decidePolicyForNavigationAction| and destroyed once any of the following // |decidePolicyForNavigationAction| and destroyed once any of the following
...@@ -67,6 +74,13 @@ using web::wk_navigation_util::IsWKInternalUrl; ...@@ -67,6 +74,13 @@ using web::wk_navigation_util::IsWKInternalUrl;
// WebController.EmptyNavigationManagerCausedByStopLoading UMA metric which // WebController.EmptyNavigationManagerCausedByStopLoading UMA metric which
// helps with diagnosing a navigation related crash (crbug.com/565457). // helps with diagnosing a navigation related crash (crbug.com/565457).
__weak WKNavigation* _stoppedWKNavigation; __weak WKNavigation* _stoppedWKNavigation;
// CertVerification errors which happened inside
// |webView:didReceiveAuthenticationChallenge:completionHandler:|.
// Key is leaf-cert/host pair. This storage is used to carry calculated
// cert status from |didReceiveAuthenticationChallenge:| to
// |didFailProvisionalNavigation:| delegate method.
std::unique_ptr<web::CertVerificationErrorsCacheType> _certVerificationErrors;
} }
// Returns the WebStateImpl from self.delegate. // Returns the WebStateImpl from self.delegate.
...@@ -77,9 +91,6 @@ using web::wk_navigation_util::IsWKInternalUrl; ...@@ -77,9 +91,6 @@ using web::wk_navigation_util::IsWKInternalUrl;
// Returns the UserInteractionState from self.delegate. // Returns the UserInteractionState from self.delegate.
@property(nonatomic, readonly, assign) @property(nonatomic, readonly, assign)
web::UserInteractionState* userInteractionState; web::UserInteractionState* userInteractionState;
// Returns the CertVerificationErrorsCacheType from self.delegate.
@property(nonatomic, readonly, assign)
web::CertVerificationErrorsCacheType* certVerificationErrors;
// Returns the CRWCertVerificationController from self.delegate. // Returns the CRWCertVerificationController from self.delegate.
@property(nonatomic, readonly, weak) @property(nonatomic, readonly, weak)
CRWCertVerificationController* certVerificationController; CRWCertVerificationController* certVerificationController;
...@@ -102,6 +113,10 @@ using web::wk_navigation_util::IsWKInternalUrl; ...@@ -102,6 +113,10 @@ using web::wk_navigation_util::IsWKInternalUrl;
// Load phase when no WebView present is 'loaded' because this represents // Load phase when no WebView present is 'loaded' because this represents
// the idle state. // the idle state.
_navigationState = web::WKNavigationState::FINISHED; _navigationState = web::WKNavigationState::FINISHED;
_certVerificationErrors =
std::make_unique<web::CertVerificationErrorsCacheType>(
kMaxCertErrorsCount);
} }
return self; return self;
} }
...@@ -684,7 +699,7 @@ using web::wk_navigation_util::IsWKInternalUrl; ...@@ -684,7 +699,7 @@ using web::wk_navigation_util::IsWKInternalUrl;
// This must be reset at the end, since code above may need information about // This must be reset at the end, since code above may need information about
// the pending load. // the pending load.
self.pendingNavigationInfo = nil; self.pendingNavigationInfo = nil;
self.certVerificationErrors->Clear(); _certVerificationErrors->Clear();
if (web::features::StorePendingItemInContext()) { if (web::features::StorePendingItemInContext()) {
// Remove the navigation to immediately get rid of pending item. // Remove the navigation to immediately get rid of pending item.
if (web::WKNavigationState::NONE != if (web::WKNavigationState::NONE !=
...@@ -716,7 +731,7 @@ using web::wk_navigation_util::IsWKInternalUrl; ...@@ -716,7 +731,7 @@ using web::wk_navigation_util::IsWKInternalUrl;
forNavigation:navigation]; forNavigation:navigation];
} }
self.certVerificationErrors->Clear(); _certVerificationErrors->Clear();
// Invariant: Every |navigation| should have a |context|. Note that violation // Invariant: Every |navigation| should have a |context|. Note that violation
// of this invariant is currently observed in production, but the cause is not // of this invariant is currently observed in production, but the cause is not
...@@ -1075,7 +1090,7 @@ using web::wk_navigation_util::IsWKInternalUrl; ...@@ -1075,7 +1090,7 @@ using web::wk_navigation_util::IsWKInternalUrl;
webView:webView webView:webView
provisionalLoad:NO]; provisionalLoad:NO];
[self.delegate navigationHandlerRemoveAllWebFrames:self]; [self.delegate navigationHandlerRemoveAllWebFrames:self];
self.certVerificationErrors->Clear(); _certVerificationErrors->Clear();
[self forgetNullWKNavigation:navigation]; [self forgetNullWKNavigation:navigation];
} }
...@@ -1125,7 +1140,7 @@ using web::wk_navigation_util::IsWKInternalUrl; ...@@ -1125,7 +1140,7 @@ using web::wk_navigation_util::IsWKInternalUrl;
- (void)webViewWebContentProcessDidTerminate:(WKWebView*)webView { - (void)webViewWebContentProcessDidTerminate:(WKWebView*)webView {
[self didReceiveWKNavigationDelegateCallback]; [self didReceiveWKNavigationDelegateCallback];
self.certVerificationErrors->Clear(); _certVerificationErrors->Clear();
self.webProcessCrashed = YES; self.webProcessCrashed = YES;
[self.delegate navigationHandlerWebProcessDidCrash:self]; [self.delegate navigationHandlerWebProcessDidCrash:self];
...@@ -1145,10 +1160,6 @@ using web::wk_navigation_util::IsWKInternalUrl; ...@@ -1145,10 +1160,6 @@ using web::wk_navigation_util::IsWKInternalUrl;
return [self.delegate userInteractionStateForNavigationHandler:self]; return [self.delegate userInteractionStateForNavigationHandler:self];
} }
- (web::CertVerificationErrorsCacheType*)certVerificationErrors {
return [self.delegate certVerificationErrorsForNavigationHandler:self];
}
- (CRWJSInjector*)JSInjector { - (CRWJSInjector*)JSInjector {
return [self.delegate JSInjectorForNavigationHandler:self]; return [self.delegate JSInjectorForNavigationHandler:self];
} }
...@@ -1489,7 +1500,7 @@ using web::wk_navigation_util::IsWKInternalUrl; ...@@ -1489,7 +1500,7 @@ using web::wk_navigation_util::IsWKInternalUrl;
policy == web::CERT_ACCEPT_POLICY_RECOVERABLE_ERROR_UNDECIDED_BY_USER; policy == web::CERT_ACCEPT_POLICY_RECOVERABLE_ERROR_UNDECIDED_BY_USER;
std::string host = std::string host =
base::SysNSStringToUTF8(challenge.protectionSpace.host); base::SysNSStringToUTF8(challenge.protectionSpace.host);
self.certVerificationErrors->Put( _certVerificationErrors->Put(
web::CertHostPair(leafCert, host), web::CertHostPair(leafCert, host),
web::CertVerificationError(is_recoverable, certStatus)); web::CertVerificationError(is_recoverable, certStatus));
} }
...@@ -1573,9 +1584,9 @@ using web::wk_navigation_util::IsWKInternalUrl; ...@@ -1573,9 +1584,9 @@ using web::wk_navigation_util::IsWKInternalUrl;
// cert decision. // cert decision.
leafCert = web::CreateCertFromChain(@[ chain.firstObject ]); leafCert = web::CreateCertFromChain(@[ chain.firstObject ]);
if (leafCert) { if (leafCert) {
auto error = self.certVerificationErrors->Get( auto error = _certVerificationErrors->Get(
{leafCert, base::SysNSStringToUTF8(host)}); {leafCert, base::SysNSStringToUTF8(host)});
bool cacheHit = error != self.certVerificationErrors->end(); bool cacheHit = error != _certVerificationErrors->end();
if (cacheHit) { if (cacheHit) {
recoverable = error->second.is_recoverable; recoverable = error->second.is_recoverable;
info.cert_status = error->second.status; info.cert_status = error->second.status;
...@@ -1962,6 +1973,7 @@ using web::wk_navigation_util::IsWKInternalUrl; ...@@ -1962,6 +1973,7 @@ using web::wk_navigation_util::IsWKInternalUrl;
self.pendingNavigationInfo.cancelled = YES; self.pendingNavigationInfo.cancelled = YES;
_safeBrowsingWarningDetectionTimer.Stop(); _safeBrowsingWarningDetectionTimer.Stop();
[self loadCancelled]; [self loadCancelled];
_certVerificationErrors->Clear();
} }
- (void)loadCancelled { - (void)loadCancelled {
......
...@@ -171,11 +171,6 @@ enum class BackForwardNavigationType { ...@@ -171,11 +171,6 @@ enum class BackForwardNavigationType {
BACK_FORWARD_NAVIGATION_TYPE_COUNT BACK_FORWARD_NAVIGATION_TYPE_COUNT
}; };
// Maximum number of errors to store in cert verification errors cache.
// Cache holds errors only for pending navigations, so the actual number of
// stored errors is not expected to be high.
const web::CertVerificationErrorsCacheType::size_type kMaxCertErrorsCount = 100;
// URLs that are fed into UIWebView as history push/replace get escaped, // URLs that are fed into UIWebView as history push/replace get escaped,
// potentially changing their format. Code that attempts to determine whether a // potentially changing their format. Code that attempts to determine whether a
// URL hasn't changed can be confused by those differences though, so method // URL hasn't changed can be confused by those differences though, so method
...@@ -246,13 +241,6 @@ GURL URLEscapedForHistory(const GURL& url) { ...@@ -246,13 +241,6 @@ GURL URLEscapedForHistory(const GURL& url) {
// Navigation Items. // Navigation Items.
CRWCertVerificationController* _certVerificationController; CRWCertVerificationController* _certVerificationController;
// CertVerification errors which happened inside
// |webView:didReceiveAuthenticationChallenge:completionHandler:|.
// Key is leaf-cert/host pair. This storage is used to carry calculated
// cert status from |didReceiveAuthenticationChallenge:| to
// |didFailProvisionalNavigation:| delegate method.
std::unique_ptr<web::CertVerificationErrorsCacheType> _certVerificationErrors;
// State of user interaction with web content. // State of user interaction with web content.
web::UserInteractionState _userInteractionState; web::UserInteractionState _userInteractionState;
} }
...@@ -449,9 +437,6 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*); ...@@ -449,9 +437,6 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
web::BrowserState* browserState = _webStateImpl->GetBrowserState(); web::BrowserState* browserState = _webStateImpl->GetBrowserState();
_certVerificationController = [[CRWCertVerificationController alloc] _certVerificationController = [[CRWCertVerificationController alloc]
initWithBrowserState:browserState]; initWithBrowserState:browserState];
_certVerificationErrors =
std::make_unique<web::CertVerificationErrorsCacheType>(
kMaxCertErrorsCount);
web::BrowsingDataRemover::FromBrowserState(browserState)->AddObserver(self); web::BrowsingDataRemover::FromBrowserState(browserState)->AddObserver(self);
web::WebFramesManagerImpl::CreateForWebState(_webStateImpl); web::WebFramesManagerImpl::CreateForWebState(_webStateImpl);
web::FindInPageManagerImpl::CreateForWebState(_webStateImpl); web::FindInPageManagerImpl::CreateForWebState(_webStateImpl);
...@@ -1746,7 +1731,6 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*); ...@@ -1746,7 +1731,6 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
- (void)abortLoad { - (void)abortLoad {
[self.webView stopLoading]; [self.webView stopLoading];
[self.navigationHandler stopLoading]; [self.navigationHandler stopLoading];
_certVerificationErrors->Clear();
} }
- (void)didFinishNavigation:(web::NavigationContextImpl*)context { - (void)didFinishNavigation:(web::NavigationContextImpl*)context {
...@@ -3659,12 +3643,6 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*); ...@@ -3659,12 +3643,6 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
return self.legacyNativeController; return self.legacyNativeController;
} }
- (web::CertVerificationErrorsCacheType*)
certVerificationErrorsForNavigationHandler:
(CRWWKNavigationHandler*)navigationHandler {
return _certVerificationErrors.get();
}
- (CRWCertVerificationController*) - (CRWCertVerificationController*)
certVerificationControllerForNavigationHandler: certVerificationControllerForNavigationHandler:
(CRWWKNavigationHandler*)navigationHandler { (CRWWKNavigationHandler*)navigationHandler {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment