Implement Reload for ORIGINAL_REQUEST_URL in NavigationManagerImpl.

ios/web/navigation/navigation_manager_impl.mm

 // Copyright 2013 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/navigation_manager_impl.h"
 
 #include <stddef.h>
 
 #include <utility>
 
@@ skipping 325 matching lines @@
 NavigationItemList NavigationManagerImpl::GetBackwardItems() const {
   return [session_controller_ backwardItems];
 }
 
 NavigationItemList NavigationManagerImpl::GetForwardItems() const {
   return [session_controller_ forwardItems];
 }
 
 void NavigationManagerImpl::Reload(ReloadType reload_type,
                                    bool check_for_reposts) {
+  // For Request Desktop/Mobile Site, we want to reload with the original

[Eugene But (OOO till 7-30), 2017/03/23 00:08:19]

Avoid "we" in the comments (go/avoidwe)

[Eugene But (OOO till 7-30), 2017/03/23 00:08:20]

This method is written in generic way, independently from Desktop UA feature. So
I don't think this method should make assumptions that it is used only for
Desktop UA feature.

[liaoyuke, 2017/03/27 16:22:42]

Done. Thank you for providing the link. :)

[liaoyuke, 2017/03/27 16:22:43]

Done.

+  // request url of the last non-redirect item (including pending item) because
+  // a server or client redirect may change the url of the current item or add a
+  // new item. For example, an user visits www.youtube.com and is then
+  // redirected to m.youtube.com, when the user clicks "Request Desktop Site",
+  // we want to request the desktop version of www.youtube.com for the user
+  // instead of m.youtube.com.
+  if (reload_type == web::ReloadType::ORIGINAL_REQUEST_URL) {
+    web::UserAgentType type =
+        [session_controller_ currentItem]->GetUserAgentType();
+    web::NavigationItem* last_non_redirect_item = nullptr;
+
+    if (GetPendingItem() && ((GetPendingItem()->GetTransitionType() &
+                              ui::PAGE_TRANSITION_IS_REDIRECT_MASK) == 0)) {
+      last_non_redirect_item = GetPendingItem();
+    } else {
+      DiscardNonCommittedItems();

[Eugene But (OOO till 7-30), 2017/03/23 00:08:19]

Is this necessary? Maybe CRWWebController already discards these during the
reload?

[liaoyuke, 2017/03/27 16:22:42]

Addressed per offline discussion.

+      DCHECK(!GetPendingItem());
+
+      int index = GetLastCommittedItemIndex();
+      while (index >= 0) {

[Eugene But (OOO till 7-30), 2017/03/23 00:08:20]

Searching for last non-redirect item can be factored in a separate function, but
the bigger question is:
how this code is different from
https://codereview.chromium.org/2756483006/patch/160001/170004?

If it's the same, then can we avoid duplicating the code? F.e. would it be
simper to fix server redirects code?

[liaoyuke, 2017/03/27 16:22:42]

|GetLastNonRedirectedItem()| in NavigationManagerUtil is meant to be replaced by
the logic here, that being said, I will delete |GetLastNonRedirectedItem| in a
separate CL.

btw, |GetLastNonRedirectedItem()| is confusing as it's unclear whether this
method counts non-history pending item or not, so I think it's better to not
expose it.

+        if (!IsRedirectItemAtIndex(index)) {
+          last_non_redirect_item = GetItemAtIndex(index);
+          break;
+        }
+
+        [session_controller_ goToItemAtIndex:(index - 1)];
+        bool item_removed_successfuly = RemoveItemAtIndex(index);

[Eugene But (OOO till 7-30), 2017/03/23 00:08:20]

Current implementation of Reload does not destroy forward items.

[liaoyuke, 2017/03/27 16:22:43]

Addressed per offline discussion.

+        DCHECK(item_removed_successfuly);
+
+        --index;
+      }
+    }
+
+    DCHECK(last_non_redirect_item);
+    last_non_redirect_item->SetURL(
+        last_non_redirect_item->GetOriginalRequestURL());
+    last_non_redirect_item->SetUserAgentType(type);
+  }
+
   delegate_->Reload();
 }
 
 void NavigationManagerImpl::CopyStateFromAndPrune(
     const NavigationManager* manager) {
   DCHECK(manager);
   CRWSessionController* other_session =
       static_cast<const NavigationManagerImpl*>(manager)->session_controller_;
   [session_controller_ copyStateFromSessionControllerAndPrune:other_session];
 }
@@ skipping 75 matching lines @@
     // The desktop user agent cannot be used for a pending navigation to an
     // app-specific URL.
     DCHECK_NE(pending_item->GetUserAgentType(), UserAgentType::NONE);
     pending_item->SetUserAgentType(UserAgentType::DESKTOP);
   } else {
     override_desktop_user_agent_for_next_pending_item_ = true;
   }
 }
 
 bool NavigationManagerImpl::IsRedirectItemAtIndex(int index) const {
-  DCHECK_GT(index, 0);
+  DCHECK_GE(index, 0);
   DCHECK_LT(index, GetItemCount());
   ui::PageTransition transition = GetItemAtIndex(index)->GetTransitionType();
   return transition & ui::PAGE_TRANSITION_IS_REDIRECT_MASK;
 }
 
 NavigationItem* NavigationManagerImpl::GetLastCommittedNonAppSpecificItem()
     const {
   int index = GetCurrentItemIndex();
   if (index == -1)
     return nullptr;
   WebClient* client = GetWebClient();
   const ScopedNavigationItemImplList& items = [session_controller_ items];
   while (index >= 0) {
     NavigationItem* item = items[index--].get();
     if (!client->IsAppSpecificURL(item->GetVirtualURL()))
       return item;
   }
   return nullptr;
 }
 
 }  // namespace web