Create new pending item if UserAgentOverrideOption is not INHERIT.

ios/web/navigation/crw_session_controller.mm

Index: ios/web/navigation/crw_session_controller.mm
diff --git a/ios/web/navigation/crw_session_controller.mm b/ios/web/navigation/crw_session_controller.mm
index 292d9fc3eb2275fe8fe977f737e694619c28b546..fbe641095d055745c67b994d39436fc8124fecd1 100644
--- a/ios/web/navigation/crw_session_controller.mm
+++ b/ios/web/navigation/crw_session_controller.mm
@@ -282,9 +282,11 @@ - (void)setBrowserState:(web::BrowserState*)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);
@@ -308,9 +310,21 @@ - (void)addPendingItem:(const GURL&)url
         PageTransitionCoreTypeIs(trans, ui::PAGE_TRANSITION_FORM_SUBMIT);
     BOOL isCurrentTransitionFormSubmit = PageTransitionCoreTypeIs(
         currentItem->GetTransitionType(), ui::PAGE_TRANSITION_FORM_SUBMIT);
+

[kkhorimoto, 2017/04/06 22:36:19]

Remove this newline.

[liaoyuke, 2017/04/07 15:49:08]

Done.

+    DCHECK(userAgentOverrideOption !=

[Eugene But (OOO till 7-30), 2017/04/06 22:53:17]

Please explain in the comments the purpose of these DCHECKs

[liaoyuke, 2017/04/07 15:49:08]

Done.

+               web::NavigationManager::UserAgentOverrideOption::DESKTOP ||
+           currentItem->GetUserAgentType() != web::UserAgentType::DESKTOP);
+    DCHECK(userAgentOverrideOption !=
+               web::NavigationManager::UserAgentOverrideOption::MOBILE ||
+           currentItem->GetUserAgentType() != web::UserAgentType::MOBILE);

[kkhorimoto, 2017/04/06 22:36:19]

Let's move these DCHECKs to the top of this function under the server transition
DCHECK, and add comments explaining them.

[liaoyuke, 2017/04/07 15:49:08]

Acknowledged.

+    BOOL hasDifferentUserAgentType =

[kkhorimoto, 2017/04/06 22:36:19]

Can we instead have a BOOL |isInheritingUserAgentType| and check
|!isInheritingUserAgentType| below?  I think that would make the intention of
this code a little clearer.

[liaoyuke, 2017/04/07 15:49:08]

Done.

+        userAgentOverrideOption !=
+        web::NavigationManager::UserAgentOverrideOption::INHERIT;
+
     BOOL shouldCreatePendingItem =

[Eugene But (OOO till 7-30), 2017/04/06 22:53:17]

This logic was already complex and got even more complicated. Do you want to
move it to a separate method like this?:

- (BOOL)shouldCreatePendingItemWithURL:transition:userAgentOverrideOption: {
  BOOL hasSameURL = currentItem->GetURL() == url;
  if (hasSameURL) {
    // explanation why pending item should not be created for the same URL
    return NO;
  }
  .. other checks...
  return YES;
}

[liaoyuke, 2017/04/07 15:49:08]

yes, that's better.

         !hasSameURL ||
-        (isPendingTransitionFormSubmit && !isCurrentTransitionFormSubmit);
+        (isPendingTransitionFormSubmit && !isCurrentTransitionFormSubmit) ||
+        hasDifferentUserAgentType;
 
     if (!shouldCreatePendingItem) {
       // Send the notification anyway, to preserve old behavior. It's unknown