This is only the first part of the refactoring. The old method was preserved

and not all call points were changed.

The second part will deal with factoring out the old OpenURL method.

Overall purpose: speed up calls, make it more flexible to plumb parameters to
the call chain, reduce code repetition.

It is necessary to add one more parameter (namely, override_encoding) to
OpenURL in order to be able to restore the page encoding upon loading the
contents. For solving this issue, it is necessary for the encoding to be
restored from the BookmarkModel.

BUG=2926

Review URL: http://codereview.chromium.org/7388007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@95013 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 102 insertions, 13 deletions

diff --git a/content/browser/tab_contents/page_navigator.cc b/content/browser/tab_contents/page_navigator.cc
new file mode 100644
index 0000000..a7bcca3
--- /dev/null
+++ b/content/browser/tab_contents/page_navigator.cc
@@ -0,0 +1,28 @@
+// Copyright (c) 2011 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.
+
+#include "content/browser/tab_contents/page_navigator.h"
+
+#include "content/common/page_transition_types.h"
+#include "webkit/glue/window_open_disposition.h"
+
+class GURL;
+
+OpenURLParams::OpenURLParams(
+    const GURL& url,
+    const GURL& referrer,
+    WindowOpenDisposition disposition,
+    PageTransition::Type transition)
+    : url(url),
+      referrer(referrer),
+      disposition(disposition),
+      transition(transition) {
+}
+
+OpenURLParams::OpenURLParams() {
+}
+
+OpenURLParams::~OpenURLParams() {
+}
+
diff --git a/content/browser/tab_contents/page_navigator.h b/content/browser/tab_contents/page_navigator.h
index 3ec6a37..64ac29d 100644
--- a/content/browser/tab_contents/page_navigator.h
+++ b/content/browser/tab_contents/page_navigator.h
@@ -10,23 +10,54 @@
 #define CONTENT_BROWSER_TAB_CONTENTS_PAGE_NAVIGATOR_H_
 #pragma once
 
+#include <string>
+
 #include "content/common/page_transition_types.h"
+#include "googleurl/src/gurl.h"
 #include "webkit/glue/window_open_disposition.h"
 
-class GURL;
 class TabContents;
 
+struct OpenURLParams {
+  OpenURLParams(const GURL& url,
+                const GURL& referrer,
+                WindowOpenDisposition disposition,
+                PageTransition::Type transition);
+  ~OpenURLParams();
+class TabContents;
+
+  // The URL/referrer to be opened.
+  GURL url;
+  GURL referrer;
+
+  // The disposition requested by the navigation source.
+  WindowOpenDisposition disposition;
+
+  // The transition type of navigation.
+  PageTransition::Type transition;
+
+  // The override encoding of the URL contents to be opened.
+  std::string override_encoding;
+
+ private:
+  OpenURLParams();
+};
+
 class PageNavigator {
  public:
-  // Opens a URL with the given disposition.  The transition specifies how this
-  // navigation should be recorded in the history system (for example, typed).
-  // Returns the TabContents the URL is opened in, or NULL if the URL wasn't
-  // opened immediately.
+  // Deprecated. Please use the one-argument variant instead.
+  // TODO(adriansc): Remove this method when refactoring changed all call sites.
   virtual TabContents* OpenURL(const GURL& url,
                                const GURL& referrer,
                                WindowOpenDisposition disposition,
                                PageTransition::Type transition) = 0;
 
+  // Opens a URL with the given disposition.  The transition specifies how this
+  // navigation should be recorded in the history system (for example, typed).
+  // Returns the TabContents the URL is opened in, or NULL if the URL wasn't
+  // opened immediately.
+  virtual TabContents* OpenURL(const OpenURLParams& params) = 0;
+
  protected:
   virtual ~PageNavigator() {}
 };
diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc
index 19357aa..65ae370 100644
--- a/content/browser/tab_contents/tab_contents.cc
+++ b/content/browser/tab_contents/tab_contents.cc
@@ -514,16 +514,21 @@ bool TabContents::NeedToFireBeforeUnload() {
       !render_view_host()->SuddenTerminationAllowed();
 }
 
+// TODO(adriansc): Remove this method once refactoring changed all call sites.
 TabContents* TabContents::OpenURL(const GURL& url,
                                   const GURL& referrer,
                                   WindowOpenDisposition disposition,
                                   PageTransition::Type transition) {
+  return OpenURL(OpenURLParams(url, referrer, disposition, transition));
+}
+
+TabContents* TabContents::OpenURL(const OpenURLParams& params) {
   if (delegate_) {
-    TabContents* new_contents =
-        delegate_->OpenURLFromTab(this, url, referrer, disposition, transition);
+    TabContents* new_contents = delegate_->OpenURLFromTab(this, params);
     // Notify observers.
     FOR_EACH_OBSERVER(TabContentsObserver, observers_,
-                      DidOpenURL(url, referrer, disposition, transition));
+                      DidOpenURL(params.url, params.referrer,
+                                 params.disposition, params.transition));
     return new_contents;
   }
   return NULL;
diff --git a/content/browser/tab_contents/tab_contents.h b/content/browser/tab_contents/tab_contents.h
index 7462cd1..bbf7694 100644
--- a/content/browser/tab_contents/tab_contents.h
+++ b/content/browser/tab_contents/tab_contents.h
@@ -254,11 +254,16 @@ class TabContents : public PageNavigator,
   // Commands ------------------------------------------------------------------
 
   // Implementation of PageNavigator.
+
+  // Deprecated. Please use the one-argument variant instead.
+  // TODO(adriansc): Remove this method once refactoring changed all call sites.
   virtual TabContents* OpenURL(const GURL& url,
                                const GURL& referrer,
                                WindowOpenDisposition disposition,
                                PageTransition::Type transition) OVERRIDE;
 
+  virtual TabContents* OpenURL(const OpenURLParams& params) OVERRIDE;
+
   // Called by the NavigationController to cause the TabContents to navigate to
   // the current pending entry. The NavigationController should be called back
   // with RendererDidNavigate on success or DiscardPendingEntry on failure.
diff --git a/content/browser/tab_contents/tab_contents_delegate.cc b/content/browser/tab_contents/tab_contents_delegate.cc
index 334ba63..2a8e703 100644
--- a/content/browser/tab_contents/tab_contents_delegate.cc
+++ b/content/browser/tab_contents/tab_contents_delegate.cc
@@ -21,6 +21,12 @@ TabContents* TabContentsDelegate::OpenURLFromTab(
     const GURL& referrer,
     WindowOpenDisposition disposition,
     PageTransition::Type transition) {
+  return OpenURLFromTab(source,
+                        OpenURLParams(url, referrer, disposition, transition));
+}
+
+TabContents* TabContentsDelegate::OpenURLFromTab(TabContents* source,
+                                                 const OpenURLParams& params) {
   return NULL;
 }
 
diff --git a/content/browser/tab_contents/tab_contents_delegate.h b/content/browser/tab_contents/tab_contents_delegate.h
index af2056e..75214cc 100644
--- a/content/browser/tab_contents/tab_contents_delegate.h
+++ b/content/browser/tab_contents/tab_contents_delegate.h
@@ -32,6 +32,7 @@ class HistoryAddPageArgs;
 }
 
 struct ContextMenuParams;
+struct OpenURLParams;
 class DownloadItem;
 class GURL;
 class HtmlDialogUIDelegate;
@@ -51,15 +52,20 @@ class TabContentsDelegate {
   //
   // A NULL source indicates the current tab (callers should probably use
   // OpenURL() for these cases which does it for you).
-  //
+
   // Returns the TabContents the URL is opened in, or NULL if the URL wasn't
   // opened immediately.
+  // Deprecated. Please use the two-arguments method instead.
+  // TODO(adriansc): Remove this method once refactoring changed all call sites.
   virtual TabContents* OpenURLFromTab(TabContents* source,
                                       const GURL& url,
                                       const GURL& referrer,
                                       WindowOpenDisposition disposition,
                                       PageTransition::Type transition);
 
+  virtual TabContents* OpenURLFromTab(TabContents* source,
+                                      const OpenURLParams& params);
+
   // Called to inform the delegate that the tab content's navigation state
   // changed. The |changed_flags| indicates the parts of the navigation state
   // that have been updated, and is any combination of the
diff --git a/content/browser/tab_contents/tab_contents_delegate_unittest.cc b/content/browser/tab_contents/tab_contents_delegate_unittest.cc
index ef4c92a9..977d28c 100644
--- a/content/browser/tab_contents/tab_contents_delegate_unittest.cc
+++ b/content/browser/tab_contents/tab_contents_delegate_unittest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "chrome/test/base/testing_profile.h"
@@ -15,11 +16,18 @@ class MockTabContentsDelegate : public TabContentsDelegate {
  public:
   virtual ~MockTabContentsDelegate() {}
 
+  // TODO(adriansc): Remove this method when refactoring changed all call sites.
+  virtual TabContents* OpenURLFromTab(
+      TabContents* source,
+      const GURL& url,
+      const GURL& referrer,
+      WindowOpenDisposition disposition,
+      PageTransition::Type transition) OVERRIDE {
+    return NULL;
+  }
+
   virtual TabContents* OpenURLFromTab(TabContents* source,
-                                      const GURL& url,
-                                      const GURL& referrer,
-                                      WindowOpenDisposition disposition,
-                                      PageTransition::Type transition) {
+                              const OpenURLParams& params) OVERRIDE {
     return NULL;
   }