[iOS] Refactoring web CRWSessionController user agent code.

ios/web/navigation/navigation_manager_impl.h

 // 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.
 
 #ifndef IOS_WEB_NAVIGATION_NAVIGATION_MANAGER_IMPL_H_
 #define IOS_WEB_NAVIGATION_NAVIGATION_MANAGER_IMPL_H_
 
 #include <stddef.h>
 
 #include <memory>
 #include <vector>
 
 #import "base/mac/scoped_nsobject.h"
 #include "base/macros.h"
 #import "ios/web/public/navigation_manager.h"
 #include "ui/base/page_transition_types.h"
 #include "url/gurl.h"
 
 @class CRWSessionController;
 
 namespace web {
 class BrowserState;
 class NavigationItem;
 struct Referrer;
 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 {
  public:
   NavigationManagerImpl();
   ~NavigationManagerImpl() override;
 
   // Setters for NavigationManagerDelegate and BrowserState.
   void SetDelegate(NavigationManagerDelegate* delegate);
   void SetBrowserState(BrowserState* browser_state);
@@ skipping 36 matching lines @@
   void OnNavigationItemCommitted();
 
   // Temporary accessors and content/ class pass-throughs.
   // TODO(stuartmorgan): Re-evaluate this list once the refactorings have
   // settled down.
   CRWSessionController* GetSessionController();
   void LoadURL(const GURL& url,
                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.
   // TODO(crbug.com/546365): Remove these methods.
   NavigationItem* GetLastUserItem() const;
   NavigationItem* GetPreviousItem() const;
 
   // Temporary method. Returns a vector of NavigationItems corresponding to
   // the SessionEntries of the uderlying CRWSessionController.
   // TODO(crbug.com/546365): Remove this method.
@@ skipping 17 matching lines @@
   int GetPendingItemIndex() const override;
   int GetLastCommittedItemIndex() const override;
   bool RemoveItemAtIndex(int index) override;
   bool CanGoBack() const override;
   bool CanGoForward() const override;
   bool CanGoToOffset(int offset) const override;
   void GoBack() override;
   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.
   // TODO(crbug.com/546197): remove once NavigationItem creation occurs in this
   // class.
   std::unique_ptr<std::vector<BrowserURLRewriter::URLRewriter>>
   GetTransientURLRewriters();
 
   // Called to reset the transient url rewriter list.
   void RemoveTransientURLRewriters();
@@ skipping 11 matching lines @@
 
  private:
   // The SessionStorageBuilder functions require access to private variables of
   // NavigationManagerImpl.
   friend SessionStorageBuilder;
 
   // Returns true if the PageTransition for the underlying navigation item at
   // |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_;
 
   // The BrowserState that is associated with this instance.
   BrowserState* browser_state_;
 
   // CRWSessionController that backs this instance.
   // TODO(stuartmorgan): Fold CRWSessionController into this class.
   base::scoped_nsobject<CRWSessionController> session_controller_;
 
   // Weak pointer to the facade delegate.
   NavigationManagerFacadeDelegate* facade_delegate_;
 
   // List of transient url rewriters added by |AddTransientURLRewriter()|.
   std::unique_ptr<std::vector<BrowserURLRewriter::URLRewriter>>
       transient_url_rewriters_;
 
   DISALLOW_COPY_AND_ASSIGN(NavigationManagerImpl);
 };
 
 }  // namespace web
 
 #endif  // IOS_WEB_NAVIGATION_NAVIGATION_MANAGER_IMPL_H_