Commit b86705bd authored by stuartmorgan's avatar stuartmorgan Committed by Commit bot

Improve page transition type heuristic in WebController

When guessing the tranistion type for a URL change:
- When possible, use specific information from the last main frame load
  request.
- Fall back to guessing based on whether there has been a touch since
  the last URL change, rather than the last document change. This
  prevents changes that happen after, e.g., a replaceState from being
  assumed to be user-initiated.

BUG=548636
TEST=On an iPhone with WKWebView enabled, follow a link on
yahoo.com, then go back via the browser back button. It should return
to the main page.

Review URL: https://codereview.chromium.org/1409033004

Cr-Commit-Position: refs/heads/master@{#357157}
parent d61641b2
...@@ -114,9 +114,9 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { ...@@ -114,9 +114,9 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
base::scoped_nsobject<CRWWKWebViewCrashDetector> _crashDetector; base::scoped_nsobject<CRWWKWebViewCrashDetector> _crashDetector;
// The actual URL of the document object (i.e., the last committed URL). // The actual URL of the document object (i.e., the last committed URL).
// TODO(stuartmorgan): Remove this in favor of just updating the session // TODO(crbug.com/549616): Remove this in favor of just updating the
// controller and treating that as authoritative. For now, this allows sharing // navigation manager and treating that as authoritative. For now, this allows
// the flow that's currently in the superclass. // sharing the flow that's currently in the superclass.
GURL _documentURL; GURL _documentURL;
// A set of script managers whose scripts have been injected into the current // A set of script managers whose scripts have been injected into the current
...@@ -174,6 +174,9 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { ...@@ -174,6 +174,9 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
// cert status from |didReceiveAuthenticationChallenge:| to // cert status from |didReceiveAuthenticationChallenge:| to
// |didFailProvisionalNavigation:| delegate method. // |didFailProvisionalNavigation:| delegate method.
scoped_ptr<CertVerificationErrorsCacheType> _certVerificationErrors; scoped_ptr<CertVerificationErrorsCacheType> _certVerificationErrors;
// YES if the user has touched the content area since the last URL change.
BOOL _touchedSinceLastURLChange;
} }
// Response's MIME type of the last known navigation. // Response's MIME type of the last known navigation.
...@@ -214,6 +217,9 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { ...@@ -214,6 +217,9 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
// Returns whether the given navigation is triggered by a user link click. // Returns whether the given navigation is triggered by a user link click.
- (BOOL)isLinkNavigation:(WKNavigationType)navigationType; - (BOOL)isLinkNavigation:(WKNavigationType)navigationType;
// Sets _documentURL to newURL, and updates any relevant state information.
- (void)setDocumentURL:(const GURL&)newURL;
// Sets value of the pendingReferrerString property. // Sets value of the pendingReferrerString property.
- (void)setPendingReferrerString:(NSString*)referrer; - (void)setPendingReferrerString:(NSString*)referrer;
...@@ -408,6 +414,12 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { ...@@ -408,6 +414,12 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
[super close]; [super close];
} }
- (void)touched:(BOOL)touched {
[super touched:touched];
if (touched)
_touchedSinceLastURLChange = YES;
}
#pragma mark - #pragma mark -
#pragma mark Testing-Only Methods #pragma mark Testing-Only Methods
...@@ -753,7 +765,7 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { ...@@ -753,7 +765,7 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
} }
_injectedScriptManagers.reset([[NSMutableSet alloc] init]); _injectedScriptManagers.reset([[NSMutableSet alloc] init]);
_crashDetector.reset([self newCrashDetectorWithWebView:_wkWebView]); _crashDetector.reset([self newCrashDetectorWithWebView:_wkWebView]);
_documentURL = [self defaultURL]; [self setDocumentURL:[self defaultURL]];
} }
- (BOOL)isLinkNavigation:(WKNavigationType)navigationType { - (BOOL)isLinkNavigation:(WKNavigationType)navigationType {
...@@ -767,6 +779,13 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { ...@@ -767,6 +779,13 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
} }
} }
- (void)setDocumentURL:(const GURL&)newURL {
if (newURL != _documentURL) {
_documentURL = newURL;
_touchedSinceLastURLChange = NO;
}
}
- (void)setPendingReferrerString:(NSString*)referrer { - (void)setPendingReferrerString:(NSString*)referrer {
_pendingReferrerString.reset([referrer copy]); _pendingReferrerString.reset([referrer copy]);
} }
...@@ -1134,14 +1153,36 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { ...@@ -1134,14 +1153,36 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
} }
- (void)registerLoadRequest:(const GURL&)url { - (void)registerLoadRequest:(const GURL&)url {
// If load request is registered via WKWebViewWebController, assume transition // Get the navigation type from the last main frame load request, and try to
// is link or client redirect as other transitions will already be registered // map that to a PageTransition.
// by web controller or delegates. WKNavigationType navigationType = _pendingNavigationTypeForMainFrame
// TODO(stuartmorgan): Remove guesswork and replace with information from ? *_pendingNavigationTypeForMainFrame
// decidePolicyForNavigationAction:. : WKNavigationTypeOther;
ui::PageTransition transition = self.userInteractionRegistered ui::PageTransition transition = ui::PAGE_TRANSITION_CLIENT_REDIRECT;
? ui::PAGE_TRANSITION_LINK switch (navigationType) {
: ui::PAGE_TRANSITION_CLIENT_REDIRECT; case WKNavigationTypeLinkActivated:
transition = ui::PAGE_TRANSITION_LINK;
break;
case WKNavigationTypeFormSubmitted:
case WKNavigationTypeFormResubmitted:
transition = ui::PAGE_TRANSITION_FORM_SUBMIT;
break;
case WKNavigationTypeBackForward:
transition = ui::PAGE_TRANSITION_FORWARD_BACK;
break;
case WKNavigationTypeReload:
transition = ui::PAGE_TRANSITION_RELOAD;
break;
case WKNavigationTypeOther:
// The "Other" type covers a variety of very different cases, which may
// or may not be the result of user actions. For now, guess based on
// whether there's been a touch since the last URL change.
// TODO(crbug.com/549301): See if this heuristic can be improved.
transition = _touchedSinceLastURLChange
? ui::PAGE_TRANSITION_LINK
: ui::PAGE_TRANSITION_CLIENT_REDIRECT;
break;
}
// The referrer is not known yet, and will be updated later. // The referrer is not known yet, and will be updated later.
const web::Referrer emptyReferrer; const web::Referrer emptyReferrer;
[self registerLoadRequest:url referrer:emptyReferrer transition:transition]; [self registerLoadRequest:url referrer:emptyReferrer transition:transition];
...@@ -1150,7 +1191,7 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { ...@@ -1150,7 +1191,7 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
- (void)URLDidChangeWithoutDocumentChange:(const GURL&)newURL { - (void)URLDidChangeWithoutDocumentChange:(const GURL&)newURL {
DCHECK(newURL == net::GURLWithNSURL([_wkWebView URL])); DCHECK(newURL == net::GURLWithNSURL([_wkWebView URL]));
DCHECK_EQ(_documentURL.host(), newURL.host()); DCHECK_EQ(_documentURL.host(), newURL.host());
_documentURL = newURL; [self setDocumentURL:newURL];
// If called during window.history.pushState or window.history.replaceState // If called during window.history.pushState or window.history.replaceState
// JavaScript evaluation, only update the document URL. This callback does not // JavaScript evaluation, only update the document URL. This callback does not
// have any information about the state object and cannot create (or edit) the // have any information about the state object and cannot create (or edit) the
...@@ -1561,7 +1602,7 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { ...@@ -1561,7 +1602,7 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
[self commitPendingReferrerString]; [self commitPendingReferrerString];
// This is the point where the document's URL has actually changed. // This is the point where the document's URL has actually changed.
_documentURL = net::GURLWithNSURL([_wkWebView URL]); [self setDocumentURL:net::GURLWithNSURL([_wkWebView URL])];
DCHECK(_documentURL == self.lastRegisteredRequestURL); DCHECK(_documentURL == self.lastRegisteredRequestURL);
[self webPageChanged]; [self webPageChanged];
......
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