Create new pending item if UserAgentOverrideOption is not INHERIT.

The original logic in |AddPendingItem| does nothing if the URL of the
pending item is the same as the last committed item, except that the
pending item is added due to form submission.

However, this doesn't work with the new Request Desktop/Mobile Site
because the new functionality requires adding a pending item with the
same URL, but different UserAgentType.

This CL re-factors |AddPendingItem| so that a pending item with the
same URL can be added successfully as long as the
UserAgentOverrideOption is not INHEIRT.

This CL also adds corresponding unit tests to test the logic.

BUG=678047

Review-Url: https://codereview.chromium.org/2794723002
Cr-Commit-Position: refs/heads/master@{#462932}

ios/web/navigation/crw_session_controller.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #import "ios/web/navigation/navigation_item_impl_list.h"
+#import "ios/web/public/navigation_manager.h"
 #include "ui/base/page_transition_types.h"
 #include "url/gurl.h"
 
@@ -77,14 +78,17 @@ struct Referrer;
 // Sets the corresponding BrowserState.
 - (void)setBrowserState:(web::BrowserState*)browserState;
 
-// Add a new item with the given url, referrer, and navigation type, making it
-// the current item. If pending item is the same as current item, this does
-// nothing. |referrer| may be nil if there isn't one. The item starts out as
-// pending, and will be lost unless |-commitPendingItem| is called.
+// Add a new item with the given url, referrer, navigation type and user agent
+// override option, making it the current item. If pending item is the same as
+// current item, this does nothing. |referrer| may be nil if there isn't one.
+// The item starts out as pending, and will be lost unless |-commitPendingItem|
+// is called.
 - (void)addPendingItem:(const GURL&)url
-              referrer:(const web::Referrer&)referrer
-            transition:(ui::PageTransition)type
-        initiationType:(web::NavigationInitiationType)initiationType;
+                   referrer:(const web::Referrer&)referrer
+                 transition:(ui::PageTransition)type
+             initiationType:(web::NavigationInitiationType)initiationType
+    userAgentOverrideOption:(web::NavigationManager::UserAgentOverrideOption)
+                                userAgentOverrideOption;
 
 // Updates the URL of the yet to be committed pending item. Useful for page
 // redirects. Does nothing if there is no pending item.

ios/web/navigation/crw_session_controller.mm

@@ -80,6 +80,21 @@ initiationType:(web::NavigationInitiationType)initiationType;
 // |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
@@ -268,9 +283,11 @@ initiationType:(web::NavigationInitiationType)initiationType;
 }
 
 - (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);
@@ -278,8 +295,33 @@ initiationType:(web::NavigationInitiationType)initiationType;
   [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())
+      _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
+                            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
@@ -288,35 +330,49 @@ initiationType:(web::NavigationInitiationType)initiationType;
   // 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 (!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;
-    }
+  if (!currentItem)
+    return YES;
+
+  // 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 isPendingTransitionReload =
+      PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_RELOAD);
+  BOOL isInheritingUserAgentType =
+      userAgentOverrideOption ==
+      web::NavigationManager::UserAgentOverrideOption::INHERIT;
+  if (isPendingTransitionReload && !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 {

ios/web/navigation/navigation_manager_impl.mm

@@ -192,7 +192,8 @@ void NavigationManagerImpl::AddPendingItem(
   [session_controller_ addPendingItem:url
                              referrer:referrer
                            transition:navigation_type
-                       initiationType:initiation_type];
+                       initiationType:initiation_type
+              userAgentOverrideOption:user_agent_override_option];
 
   // Set the user agent type for web URLs.
   NavigationItem* pending_item = GetPendingItem();

ios/web/net/crw_ssl_status_updater_unittest.mm

@@ -109,10 +109,12 @@ class CRWSSLStatusUpdaterTest : public web::WebTest {
                                            navigationItems:std::move(nav_items)
                                     lastCommittedItemIndex:0]);
     [session_controller
-        addPendingItem:GURL(item_url_spec)
-              referrer:Referrer()
-            transition:ui::PAGE_TRANSITION_LINK
-        initiationType:web::NavigationInitiationType::USER_INITIATED];
+                 addPendingItem:GURL(item_url_spec)
+                       referrer:Referrer()
+                     transition:ui::PAGE_TRANSITION_LINK
+                 initiationType:web::NavigationInitiationType::USER_INITIATED
+        userAgentOverrideOption:NavigationManager::UserAgentOverrideOption::
+                                    INHERIT];
     [session_controller commitPendingItem];
 
     return session_controller.autorelease();