[iOS] Refactoring web CRWSessionController user agent code.

ios/web/navigation/navigation_manager_impl.h

Index: ios/web/navigation/navigation_manager_impl.h
diff --git a/ios/web/navigation/navigation_manager_impl.h b/ios/web/navigation/navigation_manager_impl.h
index cf1b29562b7eca3487604784d65e6a613354ca0e..11663bd8a2d402c64e304fc04f87860d430cc48d 100644
--- a/ios/web/navigation/navigation_manager_impl.h
+++ b/ios/web/navigation/navigation_manager_impl.h
@@ -26,6 +26,20 @@ class NavigationManagerDelegate;
 class NavigationManagerFacadeDelegate;
 class SessionStorageBuilder;
 
+// Defines the ways how a pending navigation can be initiated.
+enum class NavigationInitiationType {

[kkhorimoto, 2017/02/17 02:26:37]

You can specify the amount of memory required to support an enum class by adding
a primitive type at the end.  I believe the default is an int, which is larger
than needed here.  How about we make USER_INITIATED = 0, and define the enum
class as

enum class NavigationInitiationType : BOOL { ....}

[Eugene But (OOO till 7-30), 2017/02/17 16:09:46]

Making USER_INITIATED 0 could lead to bugs. 0 is a default value in Objective-C
and variable can be 0, because it was not initialized. USER_INITIATED is more
privileged navigation, so I'd avoid initializing it with 0. Kurt, why do you
think this should start with 0?

[liaoyuke, 2017/02/17 17:44:30]

I don't understand how this could lead to bugs, enum class is type safe, so
there is no way you can compare NavigationInitiationType to any native types in
Objective-C or C++. Correct me if I'm wrong.

[Eugene But (OOO till 7-30), 2017/02/17 18:30:47]

The following code will lead to URL spoofing, because default value for
NavigationInitiationType is 0:
enum class NavigationInitiationType {
  USER_INITIATED = 0,
  RENDERER_INITIATED,
}
NavigationInitiationType type;
AddPendingItem(url, referrer, PAGE_TRANSITION_LINK, type)

[kkhorimoto, 2017/02/17 18:37:40]

I suggested making it 0 so that it fits in the BOOL type when the enum class is
evaluated and translated into its underlying representation.  However, since
BOOLs are actually just shorts, I guess that wasn't really a valid concern.  I'm
not sure about how ObjC objects would initialize an enum class with no zero
value due to Yuke's point about enum class type safety.  However, since the data
is eventually owned by NavigationItemImpl, this shouldn't be a concern.  In
fact, we should just be storing the enum class directly in the
NavigationItemImpl, which should alleviate these issues.

[kkhorimoto, 2017/02/17 18:41:33]

I'm not sure "NavigationInitiationType type;" with no initial value would work
for enum classes, as there's no implicit conversion between their numerical
representations and the enum class value. I think it's okay to leave
USER_INITIATED = 1, but we should be storing the enum class value directly in
NavigationItemImpl rather than converting to a  bool.

+  // Navigation was initiated by actual user action.
+  USER_INITIATED = 1,
+
+  // Navigation was initiated by renderer. Examples of renderer-initiated
+  // navigations include:
+  //  * <a> link click
+  //  * changing window.location.href
+  //  * redirect via the <meta http-equiv="refresh"> tag
+  //  * using window.history.pushState
+  RENDERER_INITIATED,
+};
+
 // Implementation of NavigationManager.
 // Generally mirrors upstream's NavigationController.
 class NavigationManagerImpl : public NavigationManager {
@@ -82,6 +96,16 @@ class NavigationManagerImpl : public NavigationManager {
                const Referrer& referrer,
                ui::PageTransition type);
 
+  // Adds a new item with the given url, referrer, navigation type, and
+  // initiation type, making it the pending item. If pending item is the same as
+  // the 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,
+                      const web::Referrer& referrer,
+                      ui::PageTransition navigation_type,
+                      NavigationInitiationType initiation_type);
+
   // Convenience accessors to get the underlying NavigationItems from the
   // SessionEntries returned from |session_controller_|'s -lastUserEntry and
   // -previousEntry methods.
@@ -119,6 +143,7 @@ class NavigationManagerImpl : public NavigationManager {
   void GoForward() override;
   void GoToIndex(int index) override;
   void Reload(bool check_for_reposts) override;
+  void OverrideDesktopUserAgentForNextPendingItem() override;
 
   // Returns the current list of transient url rewriters, passing ownership to
   // the caller.
@@ -150,6 +175,12 @@ class NavigationManagerImpl : public NavigationManager {
   // |index| has ui::PAGE_TRANSITION_IS_REDIRECT_MASK.
   bool IsRedirectItemAtIndex(int index) const;
 
+  // If true, override navigation item's useDesktopUserAgent flag and always
+  // create the pending entry using the desktop user agent.
+  // TODO(crbug.com/692303): Remove this when overriding the user agent doesn't
+  // create a new NavigationItem.
+  bool override_desktop_user_agent_for_next_pending_item_;
+
   // The primary delegate for this manager.
   NavigationManagerDelegate* delegate_;