(Set)IsOverridingUserAgent should be called on VisibleItem

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 58 matching lines @@
 
    // Stores the certificate policies decided by the user.
   CRWSessionCertificatePolicyManager* _sessionCertificatePolicyManager;
 
   // The timestamp of the last time this tab is visited, represented in time
   // interval since 1970.
   NSTimeInterval _lastVisitedTimestamp;
 
   // If |YES|, override |currentEntry.useDesktopUserAgent| and create the
   // pending entry using the desktop user agent.
   BOOL _useDesktopUserAgentForNextPendingItem;

[kkhorimoto, 2017/02/22 01:08:40]

I missed this while reviewing your other CL, but can you remove this since it's
no longer used?

[liaoyuke, 2017/02/22 01:37:43]

Done.

 
   // The browser state associated with this CRWSessionController;
   web::BrowserState* _browserState;  // weak
 
   // Time smoother for navigation entry timestamps; see comment in
   // navigation_controller_impl.h
   web::TimeSmoother _timeSmoother;
 }
 
 // Redefine as readwrite.
@@ skipping 399 matching lines @@
   DCHECK(navigationItem);
   navigationItem->SetTimestamp(
       _timeSmoother.GetSmoothedTime(base::Time::Now()));
 }
 
 - (void)pushNewItemWithURL:(const GURL&)URL
                stateObject:(NSString*)stateObject
                 transition:(ui::PageTransition)transition {
   DCHECK(![self pendingEntry]);
   DCHECK([self currentEntry]);
-  web::NavigationItem* currentItem = [self currentEntry].navigationItem;
+  web::NavigationItem* visibleItem = [self visibleEntry].navigationItem;
   CHECK(web::history_state_util::IsHistoryStateChangeValid(
-      currentItem->GetURL(), URL));

[liaoyuke, 2017/02/22 00:12:32]

Confirmed with Kurt and it is safe to change all currentItem to visibleItem.

[Eugene But (OOO till 7-30), 2017/02/22 21:57:41]

Why is this visible? Should this be lastCommittedItem instead?

[kkhorimoto, 2017/02/22 22:54:15]

Since a page showing an interstitial can't do a pushState, we know that if both
the DCHECKs above pass, then the visible item is equal to the last committed
item.  I guess we should just use the lastCommittedItem here to avoid doing
unnecessary logic in GetVisibleItem.

[Eugene But (OOO till 7-30), 2017/02/23 00:54:23]

Yuke, please address this comment.

[Eugene But (OOO till 7-30), 2017/02/23 02:36:59]

Yuke?

[liaoyuke, 2017/02/23 17:50:42]

Sorry, missed this one. Done!

-  web::Referrer referrer(currentItem->GetURL(), web::ReferrerPolicyDefault);
+      visibleItem->GetURL(), URL));
+  web::Referrer referrer(visibleItem->GetURL(), web::ReferrerPolicyDefault);
 
   base::scoped_nsobject<CRWSessionEntry> pushedEntry([self
       sessionEntryWithURL:URL
                  referrer:referrer
                transition:transition
            initiationType:web::NavigationInitiationType::USER_INITIATED]);
 
   web::NavigationItemImpl* pushedItem = [pushedEntry navigationItemImpl];
-  pushedItem->SetIsOverridingUserAgent(currentItem->IsOverridingUserAgent());
+  pushedItem->SetIsOverridingUserAgent(visibleItem->IsOverridingUserAgent());
   pushedItem->SetSerializedStateObject(stateObject);
   pushedItem->SetIsCreatedFromPushState(true);
   web::SSLStatus& sslStatus = [self currentEntry].navigationItem->GetSSL();
   pushedEntry.get().navigationItem->GetSSL() = sslStatus;
 
   [self clearForwardItems];
   // Add the new entry at the end.
   [_entries addObject:pushedEntry];
   _previousNavigationIndex = _currentNavigationIndex;
   self.currentNavigationIndex = [_entries count] - 1;
@@ skipping 203 matching lines @@
 }
 
 - (web::NavigationItemList)itemListForEntryList:(NSArray*)entries {
   web::NavigationItemList list(entries.count);
   for (size_t index = 0; index < entries.count; ++index)
     list[index] = [entries[index] navigationItem];
   return list;
 }
 
 @end