Create new pending item if UserAgentOverrideOption is not INHERIT.

ios/web/navigation/crw_session_controller.mm

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ios/web/navigation/crw_session_controller.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <utility>
@@ skipping 62 matching lines @@
 // Creates a NavigationItemImpl with the specified properties.
 - (std::unique_ptr<web::NavigationItemImpl>)
    itemWithURL:(const GURL&)url
       referrer:(const web::Referrer&)referrer
     transition:(ui::PageTransition)transition
 initiationType:(web::NavigationInitiationType)initiationType;
 // Returns YES if the PageTransition for the underlying navigationItem at
 // |index| in |items| has ui::PAGE_TRANSITION_IS_REDIRECT_MASK.
 - (BOOL)isRedirectTransitionForItemAtIndex:(size_t)index;
 
+// Should create a new pending item if the new pending item is not a duplicate
+// of the last added or commited item. Returns YES if one of the following rules
+// apply:
+// 1. There is no last added or committed item.
+// 2. The new item has different url from the last added or commited item.
+// 3. Url is the same, but the new item is a form submission resulted from the
+//    last added or committed item.
+// 4. Url is the same, but new item is a reload with different user agent type
+//    resulted from last added or commited item.
+- (BOOL)shouldCreatePendingItemWithURL:(const GURL&)url
+                            transition:(ui::PageTransition)transition
+               userAgentOverrideOption:
+                   (web::NavigationManager::UserAgentOverrideOption)
+                       userAgentOverrideOption;
+
 @end
 
 @implementation CRWSessionController
 
 @synthesize lastCommittedItemIndex = _lastCommittedItemIndex;
 @synthesize previousItemIndex = _previousItemIndex;
 @synthesize pendingItemIndex = _pendingItemIndex;
 
 - (instancetype)initWithBrowserState:(web::BrowserState*)browserState {
   self = [super init];
@@ skipping 168 matching lines @@
   }
 }
 
 - (void)setBrowserState:(web::BrowserState*)browserState {
   _browserState = browserState;
   DCHECK(!_navigationManager ||
          _navigationManager->GetBrowserState() == _browserState);
 }
 
 - (void)addPendingItem:(const GURL&)url
-              referrer:(const web::Referrer&)ref
-            transition:(ui::PageTransition)trans
-        initiationType:(web::NavigationInitiationType)initiationType {
+                   referrer:(const web::Referrer&)ref
+                 transition:(ui::PageTransition)trans
+             initiationType:(web::NavigationInitiationType)initiationType
+    userAgentOverrideOption:(web::NavigationManager::UserAgentOverrideOption)
+                                userAgentOverrideOption {
   // Server side redirects are handled by updating existing pending item instead
   // of adding a new item.
   DCHECK((trans & ui::PAGE_TRANSITION_SERVER_REDIRECT) == 0);
 
   [self discardTransientItem];
   self.pendingItemIndex = -1;
 
-  // Don't create a new item if it's already the same as the current item,
-  // allowing this routine to be called multiple times in a row without issue.
+  if (![self shouldCreatePendingItemWithURL:url
+                                 transition:trans
+                    userAgentOverrideOption:userAgentOverrideOption]) {
+    // Send the notification anyway, to preserve old behavior. It's unknown
+    // whether anything currently relies on this, but since both this whole
+    // hack and the content facade will both be going away, it's not worth
+    // trying to unwind.
+    if (_navigationManager && _navigationManager->GetFacadeDelegate())

[Eugene But (OOO till 7-30), 2017/04/07 16:16:40]

Kurt, do we still have "FacadeDelegate"? I suspect that we can throw this code
away :)

[kkhorimoto, 2017/04/07 17:18:11]

Nope, I've been meaning to get rid of this code for a while.  I'll set aside
some time to do it today because I'm kinda tired of having to maintain code that
isn't actually used :)

[liaoyuke, 2017/04/07 17:33:04]

Sounds good, and btw, OnNavigationItemPruned doesn't seem to be useful, we had
some code that removes navigation items but doesn't call OnNavigationItemPruned,
and nothing bad happened.

+      _navigationManager->GetFacadeDelegate()->OnNavigationItemPending();
+    return;
+  }
+
+  _pendingItem = [self itemWithURL:url
+                          referrer:ref
+                        transition:trans
+                    initiationType:initiationType];
+
+  if (_navigationManager && _navigationManager->GetFacadeDelegate())
+    _navigationManager->GetFacadeDelegate()->OnNavigationItemPending();
+  DCHECK_EQ(-1, self.pendingItemIndex);
+}
+
+- (BOOL)shouldCreatePendingItemWithURL:(const GURL&)url

[Eugene But (OOO till 7-30), 2017/04/07 16:16:40]

s/url/URL

[liaoyuke, 2017/04/07 17:33:04]

Done.

+                            transition:(ui::PageTransition)transition
+               userAgentOverrideOption:
+                   (web::NavigationManager::UserAgentOverrideOption)
+                       userAgentOverrideOption {
   // Note: CRWSessionController currently has the responsibility to distinguish
   // between new navigations and history stack navigation, hence the inclusion
   // of specific transiton type logic here, in order to make it reliable with
   // real-world observed behavior.
   // TODO(crbug.com/676129): Fix the way changes are detected/reported elsewhere
   // in the web layer so that this hack can be removed.
   // Remove the workaround code from -presentSafeBrowsingWarningForResource:.
   web::NavigationItemImpl* currentItem = self.currentItem;
-  if (currentItem) {
-    BOOL hasSameURL = currentItem->GetURL() == url;
-    BOOL isPendingTransitionFormSubmit =
-        PageTransitionCoreTypeIs(trans, ui::PAGE_TRANSITION_FORM_SUBMIT);
-    BOOL isCurrentTransitionFormSubmit = PageTransitionCoreTypeIs(
-        currentItem->GetTransitionType(), ui::PAGE_TRANSITION_FORM_SUBMIT);
-    BOOL shouldCreatePendingItem =
-        !hasSameURL ||
-        (isPendingTransitionFormSubmit && !isCurrentTransitionFormSubmit);
+  if (!currentItem)
+    return YES;
 
-    if (!shouldCreatePendingItem) {
-      // Send the notification anyway, to preserve old behavior. It's unknown
-      // whether anything currently relies on this, but since both this whole
-      // hack and the content facade will both be going away, it's not worth
-      // trying to unwind.
-      if (_navigationManager && _navigationManager->GetFacadeDelegate())
-        _navigationManager->GetFacadeDelegate()->OnNavigationItemPending();
-      return;
-    }
+  // User agent override option should always be different from the user agent
+  // type of the pending item, or the last committed item if pending doesn't
+  // exist.
+  DCHECK(userAgentOverrideOption !=
+             web::NavigationManager::UserAgentOverrideOption::DESKTOP ||
+         currentItem->GetUserAgentType() != web::UserAgentType::DESKTOP);
+  DCHECK(userAgentOverrideOption !=
+             web::NavigationManager::UserAgentOverrideOption::MOBILE ||
+         currentItem->GetUserAgentType() != web::UserAgentType::MOBILE);
+
+  BOOL hasSameURL = self.currentItem->GetURL() == url;
+  if (!hasSameURL) {
+    // Different url indicates that it's not a duplicate item.
+    return YES;
   }
 
-  _pendingItem = [self itemWithURL:url
-                          referrer:ref
-                        transition:trans
-                    initiationType:initiationType];
+  BOOL isPendingTransitionFormSubmit =
+      PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_FORM_SUBMIT);
+  BOOL isCurrentTransitionFormSubmit = PageTransitionCoreTypeIs(
+      currentItem->GetTransitionType(), ui::PAGE_TRANSITION_FORM_SUBMIT);
+  if (isPendingTransitionFormSubmit && !isCurrentTransitionFormSubmit) {
+    // |isPendingTransitionFormSubmit| indicates that the new item is a form
+    // submission resulted from the last added or commited item, and
+    // |!isCurrentTransitionFormSubmit| shows that the form submission is not
+    // counted multiple times.
+    return YES;
+  }
 
-  if (_navigationManager && _navigationManager->GetFacadeDelegate())
-    _navigationManager->GetFacadeDelegate()->OnNavigationItemPending();
-  DCHECK_EQ(-1, self.pendingItemIndex);
+  BOOL isInheritingUserAgentType =
+      userAgentOverrideOption ==
+      web::NavigationManager::UserAgentOverrideOption::INHERIT;
+  if (!isInheritingUserAgentType) {
+    // Overriding user agent type to MOBILE or DESKTOP indicates that the new
+    // new item is a reload with different user agent type.
+    return YES;
+  }
+
+  return NO;
 }
 
 - (void)updatePendingItem:(const GURL&)url {
   // If there is no pending item, navigation is probably happening within the
   // session history. Don't modify the item list.
   web::NavigationItemImpl* item = self.pendingItem;
   if (!item)
     return;
 
   if (url != item->GetURL()) {
@@ skipping 285 matching lines @@
   return item;
 }
 
 - (BOOL)isRedirectTransitionForItemAtIndex:(size_t)index {
   DCHECK_LT(index, self.items.size());
   ui::PageTransition transition = self.items[index]->GetTransitionType();
   return (transition & ui::PAGE_TRANSITION_IS_REDIRECT_MASK) ? YES : NO;
 }
 
 @end