Reland of (Set)IsOverridingUserAgent should be called on VisibleItem

ios/web/web_state/ui/crw_web_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/web_state/ui/crw_web_controller.h"
 
 #import <WebKit/WebKit.h>
 
 #import <objc/runtime.h>
 #include <stddef.h>
@@ skipping 440 matching lines @@
 // This method can change the state of the CRWWebController, as it will display
 // an error if the returned URL is not reliable from a security point of view.
 // Note that this method is expensive, so it should always be cached locally if
 // it's needed multiple times in a method.
 @property(nonatomic, readonly) GURL currentURL;
 // Returns the referrer for the current page.
 @property(nonatomic, readonly) web::Referrer currentReferrer;
 
 // Returns YES if the user interacted with the page recently.
 @property(nonatomic, readonly) BOOL userClickedRecently;
-// Returns whether the desktop user agent should be used when setting the user
-// agent.
-@property(nonatomic, readonly) BOOL useDesktopUserAgent;
+
+// Whether or not desktop user agent is used for the currentItem.
+@property(nonatomic, readonly) BOOL usesDesktopUserAgent;

[kkhorimoto, 2017/02/24 19:21:27]

Again, I think that it would be better to fold this logic directly into
|-webViewWithConfiguration:|.  The concept of a "current item" is going away in
the refactor, so exposing a property whose semantics are tightly knit with that
API is a bad idea.  When future users of the NavigationManager API need that
functionality, they will need to implement it themselves (and add a comment
explaining why that logic is needed, rather than using an existing
NavigationItem getter). Exposing it in a property like this invites misuse, as
someone working on this file in the future may use it without fully thinking
through whether the flag from the "current item" is needed.

[Eugene But (OOO till 7-30), 2017/02/24 19:49:18]

This is existing property which will be fully removed. There is no need to pass
self.usesDesktopUserAgent to BuildWKWebView, this logic is UIWebView cruft. With
UIWebView the only way to set UserAgent was changing NSUserDefaults before
UIWebView creation. WKWebView has API to set user agent, so there is no need to
recreate it when desktop UA is requested. I think we should leave this code as
it is because we will throw it away anyway.

 
 // Facade for Mojo API.
 @property(nonatomic, readonly) web::MojoFacade* mojoFacade;
 
 // Updates Desktop User Agent and calls webWillFinishHistoryNavigationFromEntry:
 // on CRWWebDelegate. TODO(crbug.com/684098): Remove this method and inline its
 // content.
 - (void)webWillFinishHistoryNavigationFromEntry:(CRWSessionEntry*)fromEntry;
 // Recreates web view if |entry| and |fromEntry| have different value for
 // IsOverridingUserAgent() flag.
@@ skipping 1868 matching lines @@
   // The page should be closed if it was initiated by the DOM and there has been
   // no user interaction with the page since the web view was created, or if
   // the page has no navigation items, as occurs when an App Store link is
   // opened from another application.
   BOOL rendererInitiatedWithoutInteraction =
       self.sessionController.openedByDOM && !_userInteractedWithWebController;
   BOOL noNavigationItems = !(self.navigationManagerImpl->GetItemCount());
   return rendererInitiatedWithoutInteraction || noNavigationItems;
 }
 
-- (BOOL)useDesktopUserAgent {
+- (BOOL)usesDesktopUserAgent {
   web::NavigationItem* item = [self currentNavItem];
   return item && item->IsOverridingUserAgent();
 }
 
 - (web::MojoFacade*)mojoFacade {
   if (!_mojoFacade) {
     service_manager::mojom::InterfaceProvider* interfaceProvider =
         _webStateImpl->GetMojoInterfaceRegistry();
     _mojoFacade.reset(new web::MojoFacade(interfaceProvider, self));
   }
@@ skipping 1855 matching lines @@
                                             scrollView:self.webScrollView]);
     [_containerView displayWebViewContentView:webViewContentView];
   }
 }
 
 - (WKWebView*)webViewWithConfiguration:(WKWebViewConfiguration*)config {
   // Do not attach the context menu controller immediately as the JavaScript
   // delegate must be specified.
   return web::BuildWKWebView(CGRectZero, config,
                              self.webStateImpl->GetBrowserState(),
-                             [self useDesktopUserAgent]);
+                             self.usesDesktopUserAgent);
 }
 
 - (void)setWebView:(WKWebView*)webView {
   DCHECK_NE(_webView.get(), webView);
 
   // Unwind the old web view.
   // TODO(eugenebut): Remove CRWWKScriptMessageRouter once crbug.com/543374 is
   // fixed.
   CRWWKScriptMessageRouter* messageRouter =
       [self webViewConfigurationProvider].GetScriptMessageRouter();
@@ skipping 1009 matching lines @@
 - (void)simulateLoadRequestWithURL:(const GURL&)URL {
   _lastRegisteredRequestURL = URL;
   _loadPhase = web::LOAD_REQUESTED;
 }
 
 - (NSString*)referrerFromNavigationAction:(WKNavigationAction*)action {
   return [action.request valueForHTTPHeaderField:kReferrerHeaderName];
 }
 
 @end