Consolidate all NavigationController::LoadURL and family functions

content/public/browser/navigation_controller.h

 // Copyright (c) 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.
 
 #ifndef CONTENT_PUBLIC_BROWSER_NAVIGATION_CONTROLLER_H_
 #define CONTENT_PUBLIC_BROWSER_NAVIGATION_CONTROLLER_H_
 
 #include <string>
 #include <vector>
 
+#include "base/memory/ref_counted_memory.h"
 #include "base/string16.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/global_request_id.h"
 #include "content/public/common/page_transition_types.h"
-
-class GURL;
+#include "content/public/common/referrer.h"
+#include "googleurl/src/gurl.h"
 
 namespace content {
 
 class BrowserContext;
 class NavigationEntry;
 class SessionStorageNamespace;
 class WebContents;
-struct Referrer;
 
 // A NavigationController maintains the back-forward list for a WebContents and
 // manages all navigation within that list.
 //
 // Each NavigationController belongs to one WebContents; each WebContents has
 // exactly one NavigationController.
 class NavigationController {
  public:
   enum ReloadType {
     NO_RELOAD,                // Normal load.
     RELOAD,                   // Normal (cache-validating) reload.
     RELOAD_IGNORING_CACHE     // Reload bypassing the cache, aka shift-reload.
   };
 
   // Creates a navigation entry and translates the virtual url to a real one.
   // This is a general call; prefer LoadURL[FromRenderer]/TransferURL below.
   // Extra headers are separated by \n.
   CONTENT_EXPORT static NavigationEntry* CreateNavigationEntry(
       const GURL& url,
       const Referrer& referrer,
       PageTransition transition,
       bool is_renderer_initiated,
       const std::string& extra_headers,
       BrowserContext* browser_context);
 
+  // Load type used in LoadURLParams.
+  enum LoadURLType {

[Charlie Reis, 2012/08/03 21:28:36]

enums and structs belong above the static CreateNavigationEntry function, I
believe.

[boliu, 2012/08/03 22:42:08]

Done. Placed new UserAgentOverrideOption enum here as well.

+    // For loads that does not fall into any types below.

[Charlie Reis, 2012/08/03 21:28:36]

nit: do not

[boliu, 2012/08/03 22:42:08]

Done.

+    LOAD_TYPE_DEFAULT,
+
+    // An http post load request initiated from browser side.
+    // The post data is passed in |browser_initiated_post_data|.
+    // Used in Android WebView.postUrl implementation.

[Charlie Reis, 2012/08/03 21:28:36]

I don't think we should talk about Android WebView uses in the public API
documentation.  Maybe we can mention it somewhere else?

[boliu, 2012/08/03 22:42:08]

Removed mentions of webview usage. I originally added it to avoid confusing
chromium developers using this api wondering about what load type to use. The
android code using this has not been upstreamed yet.

+    LOAD_TYPE_BROWSER_INITIATED_HTTP_POST,
+
+    // Loads a 'data:' scheme URL with specified base URL and a history entry
+    // URL. This is only safe to be used for browser-initiated data: URL
+    // navigations, since it shows arbitrary content as if it comes from
+    // |virtual_url|. Used in Android WebView.loadDataWithBaseURL
+    // implementation.
+    LOAD_TYPE_DATA
+  };
+
+  // Extra optional parameters for LoadURLWithParams.
+  CONTENT_EXPORT struct LoadURLParams {
+    // The url to load. This field is required.
+    const GURL url;
+
+    // See LoadURLType comments above.
+    LoadURLType load_type;
+
+    // PageTransition for this load. See PageTransition for details.
+    // Note the default value in constructor below.
+    PageTransition transition_type;
+
+    // Referrer for this load. Empty if none.
+    Referrer referrer;
+
+    // Extra headers for this load, separated by \n.
+    std::string extra_headers;
+
+    // True for renderer-initiated navigations. This is
+    // important for tracking whether to display pending URLs.
+    bool is_renderer_initiated;
+
+    // If true, then |is_overriding_user_agent| is ignored and the user agent
+    // override boolean is inherited from previous navigations.
+    // Note the default value of this is true.
+    bool should_inherit_user_agent_override;
+
+    // Ignored if |should_inherit_user_agent_override| is true. Otherwise,
+    // sets the user agent override boolean of the load to this value.
+    bool is_overriding_user_agent;
+
+    // Marks the new navigation as being transferred from one RVH to another.
+    // In this case the browser can recycle the old request once the new
+    // renderer wants to navigate. Identifies the request ID of the old request.
+    GlobalRequestID transferred_global_request_id;
+
+    // Used in LOAD_TYPE_DATA loads only. Used for specifying a base URL
+    // for pages loaded via data URLs.
+    GURL base_url_for_data_url;
+
+    // Used in LOAD_TYPE_DATA loads only. URL displayed to the user for
+    // data loads.
+    GURL virtual_url;

[Charlie Reis, 2012/08/03 21:28:36]

virtual_url has a different meaning in NavigationEntry (used outside of
LOAD_TYPE_DATA), which makes this confusing.  Please use either
virtual_url_for_data_url or history_url_for_data_url.

[boliu, 2012/08/03 22:42:08]

Renamed to virtual_url_for_data_url.

+
+    // Used in LOAD_TYPE_BROWSER_INITIATED_HTTP_POST loads only. Carrys the

[gone, 2012/08/03 20:17:37]

nit: Carrys -> Carries

[boliu, 2012/08/03 22:42:08]

Done.

+    // post data of the load.
+    base::RefCountedMemory* browser_initiated_post_data;
+
+    LoadURLParams(const GURL& url)
+        : url(url),
+          load_type(LOAD_TYPE_DEFAULT),
+          transition_type(PAGE_TRANSITION_LINK),
+          is_renderer_initiated(false),
+          should_inherit_user_agent_override(true),
+          is_overriding_user_agent(false),
+          browser_initiated_post_data(NULL) {
+    }
+  };
+
   // Disables checking for a repost and prompting the user. This is used during
   // testing.
   CONTENT_EXPORT static void DisablePromptOnRepost();
 
   virtual ~NavigationController() {}
 
   // Returns the web contents associated with this controller. It can never be
   // NULL.
   virtual WebContents* GetWebContents() const = 0;
 
@@ skipping 92 matching lines @@
 
   // New navigations -----------------------------------------------------------
 
   // Loads the specified URL, specifying extra http headers to add to the
   // request.  Extra headers are separated by \n.
   virtual void LoadURL(const GURL& url,
                        const Referrer& referrer,
                        PageTransition type,
                        const std::string& extra_headers) = 0;
 
-  // Same as LoadURL, but for renderer-initiated navigations.  This state is
-  // important for tracking whether to display pending URLs.
-  virtual void LoadURLFromRenderer(const GURL& url,
-                                   const Referrer& referrer,
-                                   PageTransition type,
-                                   const std::string& extra_headers) = 0;
-
-  // Same as LoadURL, but allows overriding the user agent of the
-  // NavigationEntry before it loads.
-  // TODO(dfalcantara): Consolidate the LoadURL* interfaces.
-  virtual void LoadURLWithUserAgentOverride(const GURL& url,
-                                            const Referrer& referrer,
-                                            PageTransition type,
-                                            bool is_renderer_initiated,
-                                            const std::string& extra_headers,
-                                            bool is_overriding_user_agent) = 0;
-
-  // Behaves like LoadURL() and LoadURLFromRenderer() but marks the new
-  // navigation as being transferred from one RVH to another. In this case the
-  // browser can recycle the old request once the new renderer wants to
-  // navigate.
-  // |transferred_global_request_id| identifies the request ID of the old
-  // request.
-  virtual void TransferURL(
-      const GURL& url,
-      const Referrer& referrer,
-      PageTransition transition,
-      const std::string& extra_headers,
-      const GlobalRequestID& transferred_global_request_id,
-      bool is_renderer_initiated) = 0;
+  // More general version of LoadURL. See comments in LoadURLParams for
+  // using |extra_params|.

[Charlie Reis, 2012/08/03 21:28:36]

nit: |params|

[boliu, 2012/08/03 22:42:08]

Done.

+  virtual void LoadURLWithParams(LoadURLParams& params) = 0;

[gone, 2012/08/03 20:17:37]

Feels like the LoadURLParams struct should only carry "extra" parameters that
aren't sent in the LoadURL call (i.e. not the url, referrer, type, or
extra_headers).  It'd probably save a few copies...

[boliu, 2012/08/03 20:46:27]

I was debating about this. And previous Patch Set 4 does exactly what you
describe here.

Problem is that all the parameters except |url| are "extra". There are code
refactored in this CL that just passes in empty strings for referrer and
extra_headers. I guess PageTransition is "required", but it has a default value
so I didn't require it in the construction of LoadULRParams.

I can make this LoadURL(GURL& url, LoadURLParams& params) to save one copy.

Charlie, do you have any thoughts on this?

[Charlie Reis, 2012/08/03 21:28:36]

I'm ok with it as is.

 
   // Loads the current page if this NavigationController was restored from
   // history and the current page has not loaded yet.
   virtual void LoadIfNecessary() = 0;
 
   // Renavigation --------------------------------------------------------------
 
   // Navigation relative to the "current entry"
   virtual bool CanGoBack() const = 0;
   virtual bool CanGoForward() const = 0;
@@ skipping 68 matching lines @@
   virtual void CopyStateFromAndPrune(NavigationController* source) = 0;
 
   // Removes all the entries except the active entry. If there is a new pending
   // navigation it is preserved.
   virtual void PruneAllButActive() = 0;
 };
 
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_BROWSER_NAVIGATION_CONTROLLER_H_