C++ readability review for lazyboy@

The changes are just code cleanups and no functional change.

This is the original CL that was submitted: https://chromiumcodereview.appspot.com/10868012.
This patch evolved initially from http://chromiumcodereview.appspot.com/10560022
I've included the files from browser_plugin/* dir.

Update:
Since the original CL became stale, I've uploaded a smaller subset from browser_plugin/ dir.
BUG=

Review URL: https://codereview.chromium.org/11312179

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

7 files changed, 72 insertions, 71 deletions

diff --git a/content/browser/browser_plugin/browser_plugin_embedder.cc b/content/browser/browser_plugin/browser_plugin_embedder.cc
index 16cef0a..1418f97 100644
--- a/content/browser/browser_plugin/browser_plugin_embedder.cc
+++ b/content/browser/browser_plugin/browser_plugin_embedder.cc
@@ -203,7 +203,7 @@ void BrowserPluginEmbedder::OnAttach(
         guest->GetWebContents(),
         web_contents(),
         extra_params);
-    guest->Initialize(static_cast<WebContentsImpl*>(web_contents()), params);
+    guest->Initialize(params, static_cast<WebContentsImpl*>(web_contents()));
   }
 }
 
diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index 4c01545..3395a42 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -64,7 +64,7 @@ class BrowserPluginGuest::PermissionRequest :
  public:
   virtual void Respond(bool should_allow, const std::string& user_input) = 0;
   virtual bool AllowedByDefault() const {
-      return false;
+    return false;
   }
  protected:
   PermissionRequest() {
@@ -298,7 +298,7 @@ static std::string RetrieveDownloadURLFromRequestId(
       ResourceDispatcherHostImpl::Get()->GetURLRequest(global_id);
   if (url_request)
     return url_request->url().possibly_invalid_spec();
-  return std::string();
+  return "";
 }
 
 }  // namespace
@@ -335,9 +335,9 @@ class BrowserPluginGuest::EmbedderWebContentsObserver
 
 BrowserPluginGuest::BrowserPluginGuest(
     int instance_id,
+    bool has_render_view,
     WebContentsImpl* web_contents,
-    BrowserPluginGuest* opener,
-    bool has_render_view)
+    BrowserPluginGuest* opener)
     : WebContentsObserver(web_contents),
       weak_ptr_factory_(this),
       embedder_web_contents_(NULL),
@@ -388,13 +388,13 @@ void BrowserPluginGuest::DestroyUnattachedWindows() {
   }
   // All pending windows should be removed from the set after Destroy() is
   // called on all of them.
-  DCHECK_EQ(0ul, pending_new_windows_.size());
+  DCHECK(pending_new_windows_.empty());
 }
 
-void BrowserPluginGuest::LoadURLWithParams(WebContents* web_contents,
-                                           const GURL& url,
+void BrowserPluginGuest::LoadURLWithParams(const GURL& url,
                                            const Referrer& referrer,
-                                           PageTransition transition_type) {
+                                           PageTransition transition_type,
+                                           WebContents* web_contents) {
   NavigationController::LoadURLParams load_url_params(url);
   load_url_params.referrer = referrer;
   load_url_params.transition_type = transition_type;
@@ -478,11 +478,8 @@ BrowserPluginGuest* BrowserPluginGuest::CreateNewGuestWindow(
       std::make_pair(new_guest, NewWindowInfo(params.url, std::string())));
 
   // Request permission to show the new window.
-  RequestNewWindowPermission(
-      new_guest->GetWebContents(),
-      params.disposition,
-      gfx::Rect(),
-      params.user_gesture);
+  RequestNewWindowPermission(params.disposition, gfx::Rect(),
+                             params.user_gesture, new_guest->GetWebContents());
 
   return new_guest;
 }
@@ -536,8 +533,8 @@ bool BrowserPluginGuest::OnMessageReceivedFromEmbedder(
 }
 
 void BrowserPluginGuest::Initialize(
-    WebContentsImpl* embedder_web_contents,
-    const BrowserPluginHostMsg_Attach_Params& params) {
+    const BrowserPluginHostMsg_Attach_Params& params,
+    WebContentsImpl* embedder_web_contents) {
   focused_ = params.focused;
   guest_visible_ = params.visible;
   guest_window_rect_ = params.resize_guest_params.view_rect;
@@ -644,7 +641,7 @@ BrowserPluginGuest* BrowserPluginGuest::Create(
   if (factory_) {
     guest = factory_->CreateBrowserPluginGuest(instance_id, web_contents);
   } else {
-    guest = new BrowserPluginGuest(instance_id, web_contents, NULL, false);
+    guest = new BrowserPluginGuest(instance_id, false, web_contents, NULL);
   }
   guest->extra_attach_params_.reset(extra_params->DeepCopy());
   web_contents->SetBrowserPluginGuest(guest);
@@ -658,12 +655,12 @@ BrowserPluginGuest* BrowserPluginGuest::Create(
 // static
 BrowserPluginGuest* BrowserPluginGuest::CreateWithOpener(
     int instance_id,
+    bool has_render_view,
     WebContentsImpl* web_contents,
-    BrowserPluginGuest* opener,
-    bool has_render_view) {
+    BrowserPluginGuest* opener) {
   BrowserPluginGuest* guest =
       new BrowserPluginGuest(
-          instance_id, web_contents, opener, has_render_view);
+          instance_id, has_render_view, web_contents, opener);
   web_contents->SetBrowserPluginGuest(guest);
   BrowserPluginGuestDelegate* delegate = NULL;
   GetContentClient()->browser()->GuestWebContentsCreated(
@@ -702,8 +699,8 @@ void BrowserPluginGuest::AddNewContents(WebContents* source,
                                         bool* was_blocked) {
   if (was_blocked)
     *was_blocked = false;
-  RequestNewWindowPermission(static_cast<WebContentsImpl*>(new_contents),
-                             disposition, initial_pos, user_gesture);
+  RequestNewWindowPermission(disposition, initial_pos, user_gesture,
+                             static_cast<WebContentsImpl*>(new_contents));
 }
 
 void BrowserPluginGuest::CanDownload(
@@ -786,7 +783,7 @@ WebContents* BrowserPluginGuest::OpenURLFromTab(WebContents* source,
   }
   if (params.disposition == CURRENT_TAB) {
     // This can happen for cross-site redirects.
-    LoadURLWithParams(source, params.url, params.referrer, params.transition);
+    LoadURLWithParams(params.url, params.referrer, params.transition, source);
     return source;
   }
 
@@ -888,10 +885,10 @@ bool BrowserPluginGuest::InAutoSizeBounds(const gfx::Size& size) const {
 }
 
 void BrowserPluginGuest::RequestNewWindowPermission(
-    WebContentsImpl* new_contents,
     WindowOpenDisposition disposition,
     const gfx::Rect& initial_bounds,
-    bool user_gesture) {
+    bool user_gesture,
+    WebContentsImpl* new_contents) {
   BrowserPluginGuest* guest = new_contents->GetBrowserPluginGuest();
   PendingWindowMap::iterator it = pending_new_windows_.find(guest);
   if (it == pending_new_windows_.end())
@@ -1124,9 +1121,8 @@ bool BrowserPluginGuest::ShouldForwardToBrowserPluginGuest(
     case BrowserPluginHostMsg_UpdateRect_ACK::ID:
       return true;
     default:
-      break;
+      return false;
   }
-  return false;
 }
 
 bool BrowserPluginGuest::OnMessageReceived(const IPC::Message& message) {
@@ -1200,7 +1196,7 @@ void BrowserPluginGuest::Attach(
   if (!name_.empty())
     params.name.clear();
 
-  Initialize(embedder_web_contents, params);
+  Initialize(params, embedder_web_contents);
 
   SendQueuedMessages();
 
@@ -1377,8 +1373,8 @@ void BrowserPluginGuest::OnNavigateGuest(
   // normal web URLs are supported.  No protocol handlers are installed for
   // other schemes (e.g., WebUI or extensions), and no permissions or bindings
   // can be granted to the guest process.
-  LoadURLWithParams(GetWebContents(), validated_url, Referrer(),
-                    PAGE_TRANSITION_AUTO_TOPLEVEL);
+  LoadURLWithParams(validated_url, Referrer(), PAGE_TRANSITION_AUTO_TOPLEVEL,
+                    GetWebContents());
 }
 
 void BrowserPluginGuest::OnPluginDestroyed(int instance_id) {
diff --git a/content/browser/browser_plugin/browser_plugin_guest.h b/content/browser/browser_plugin/browser_plugin_guest.h
index 9ccf861..4633e9e 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.h
+++ b/content/browser/browser_plugin/browser_plugin_guest.h
@@ -102,9 +102,9 @@ class CONTENT_EXPORT BrowserPluginGuest
 
   static BrowserPluginGuest* CreateWithOpener(
       int instance_id,
+      bool has_render_view,
       WebContentsImpl* web_contents,
-      BrowserPluginGuest* opener,
-      bool has_render_view);
+      BrowserPluginGuest* opener);
 
   // Called when the embedder WebContents is destroyed to give the
   // BrowserPluginGuest an opportunity to clean up after itself.
@@ -121,20 +121,10 @@ class CONTENT_EXPORT BrowserPluginGuest
   // within an embedder.
   int instance_id() const { return instance_id_; }
 
-  // Overrides factory for testing. Default (NULL) value indicates regular
-  // (non-test) environment.
-  static void set_factory_for_testing(BrowserPluginHostFactory* factory) {
-    BrowserPluginGuest::factory_ = factory;
-  }
-
   bool OnMessageReceivedFromEmbedder(const IPC::Message& message);
 
-  void Initialize(WebContentsImpl* embedder_web_contents,
-                  const BrowserPluginHostMsg_Attach_Params& params);
-
-  void set_guest_hang_timeout_for_testing(const base::TimeDelta& timeout) {
-    guest_hang_timeout_ = timeout;
-  }
+  void Initialize(const BrowserPluginHostMsg_Attach_Params& params,
+                  WebContentsImpl* embedder_web_contents);
 
   WebContentsImpl* embedder_web_contents() const {
     return embedder_web_contents_;
@@ -250,7 +240,7 @@ class CONTENT_EXPORT BrowserPluginGuest
   virtual void SendMessageToEmbedder(IPC::Message* msg);
 
   // Returns whether the guest is attached to an embedder.
-  bool attached() const { return !!embedder_web_contents_; }
+  bool attached() const { return embedder_web_contents_ != NULL; }
 
   // Attaches this BrowserPluginGuest to the provided |embedder_web_contents|
   // and initializes the guest with the provided |params|. Attaching a guest
@@ -297,6 +287,12 @@ class CONTENT_EXPORT BrowserPluginGuest
                                   bool should_allow,
                                   const std::string& user_input);
 
+  // Overrides factory for testing. Default (NULL) value indicates regular
+  // (non-test) environment.
+  static void set_factory_for_testing(BrowserPluginHostFactory* factory) {
+    BrowserPluginGuest::factory_ = factory;
+  }
+
  private:
   class EmbedderWebContentsObserver;
   friend class TestBrowserPluginGuest;
@@ -310,19 +306,35 @@ class CONTENT_EXPORT BrowserPluginGuest
   class PermissionRequest;
   class PointerLockRequest;
 
+  // Tracks the name, and target URL of the new window and whether or not it has
+  // changed since the WebContents has been created and before the new window
+  // has been attached to a BrowserPlugin. Once the first navigation commits, we
+  // no longer track this information.
+  struct NewWindowInfo {
+    bool changed;
+    GURL url;
+    std::string name;
+    NewWindowInfo(const GURL& url, const std::string& name) :
+        changed(false),
+        url(url),
+        name(name) {}
+  };
+
+  // BrowserPluginGuest is a WebContentsObserver of |web_contents| and
+  // |web_contents| has to stay valid for the lifetime of BrowserPluginGuest.
   BrowserPluginGuest(int instance_id,
+                     bool has_render_view,
                      WebContentsImpl* web_contents,
-                     BrowserPluginGuest* opener,
-                     bool has_render_view);
+                     BrowserPluginGuest* opener);
 
   // Destroy unattached new windows that have been opened by this
   // BrowserPluginGuest.
   void DestroyUnattachedWindows();
 
-  void LoadURLWithParams(WebContents* web_contents,
-                         const GURL& url,
+  void LoadURLWithParams(const GURL& url,
                          const Referrer& referrer,
-                         PageTransition transition_type);
+                         PageTransition transition_type,
+                         WebContents* web_contents);
 
   // Bridge IDs correspond to a geolocation request. This method will remove
   // the bookkeeping for a particular geolocation request associated with the
@@ -350,10 +362,10 @@ class CONTENT_EXPORT BrowserPluginGuest
 
   bool InAutoSizeBounds(const gfx::Size& size) const;
 
-  void RequestNewWindowPermission(WebContentsImpl* new_contents,
-                                  WindowOpenDisposition disposition,
+  void RequestNewWindowPermission(WindowOpenDisposition disposition,
                                   const gfx::Rect& initial_bounds,
-                                  bool user_gesture);
+                                  bool user_gesture,
+                                  WebContentsImpl* new_contents);
 
   // Message handlers for messages from embedder.
 
@@ -505,19 +517,6 @@ class CONTENT_EXPORT BrowserPluginGuest
   gfx::Size max_auto_size_;
   gfx::Size min_auto_size_;
 
-  // Tracks the name, and target URL of the new window and whether or not it has
-  // changed since the WebContents has been created and before the new window
-  // has been attached to a BrowserPlugin. Once the first navigation commits, we
-  // no longer track this information.
-  struct NewWindowInfo {
-    bool changed;
-    GURL url;
-    std::string name;
-    NewWindowInfo(const GURL& url, const std::string& name) :
-        changed(false),
-        url(url),
-        name(name) {}
-  };
   typedef std::map<BrowserPluginGuest*, NewWindowInfo> PendingWindowMap;
   PendingWindowMap pending_new_windows_;
   base::WeakPtr<BrowserPluginGuest> opener_;
diff --git a/content/browser/browser_plugin/browser_plugin_host_browsertest.cc b/content/browser/browser_plugin/browser_plugin_host_browsertest.cc
index 417d916..17a1bc4 100644
--- a/content/browser/browser_plugin/browser_plugin_host_browsertest.cc
+++ b/content/browser/browser_plugin/browser_plugin_host_browsertest.cc
@@ -145,9 +145,9 @@ class TestShortHangTimeoutGuestFactory : public TestBrowserPluginHostFactory {
  public:
   virtual BrowserPluginGuest* CreateBrowserPluginGuest(
       int instance_id, WebContentsImpl* web_contents) OVERRIDE {
-    BrowserPluginGuest* guest =
+    TestBrowserPluginGuest* guest =
         new TestBrowserPluginGuest(instance_id, web_contents);
-    guest->set_guest_hang_timeout_for_testing(TestTimeouts::tiny_timeout());
+    guest->set_guest_hang_timeout(TestTimeouts::tiny_timeout());
     return guest;
   }
 
diff --git a/content/browser/browser_plugin/test_browser_plugin_guest.cc b/content/browser/browser_plugin/test_browser_plugin_guest.cc
index 24a2341..63237b3 100644
--- a/content/browser/browser_plugin/test_browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/test_browser_plugin_guest.cc
@@ -16,7 +16,7 @@ class BrowserPluginGuest;
 TestBrowserPluginGuest::TestBrowserPluginGuest(
     int instance_id,
     WebContentsImpl* web_contents)
-    : BrowserPluginGuest(instance_id, web_contents, NULL, false),
+    : BrowserPluginGuest(instance_id, false, web_contents, NULL),
       update_rect_count_(0),
       damage_buffer_call_count_(0),
       exit_observed_(false),
diff --git a/content/browser/browser_plugin/test_browser_plugin_guest.h b/content/browser/browser_plugin/test_browser_plugin_guest.h
index 53f7f8c..a66e86b 100644
--- a/content/browser/browser_plugin/test_browser_plugin_guest.h
+++ b/content/browser/browser_plugin/test_browser_plugin_guest.h
@@ -65,6 +65,10 @@ class TestBrowserPluginGuest : public BrowserPluginGuest {
   // Waits until UpdateRect with a particular |view_size| is observed.
   void WaitForViewSize(const gfx::Size& view_size);
 
+  void set_guest_hang_timeout(const base::TimeDelta& timeout) {
+    guest_hang_timeout_ = timeout;
+  }
+
  private:
   // Overridden methods from BrowserPluginGuest to intercept in test objects.
   virtual void SendMessageToEmbedder(IPC::Message* msg) OVERRIDE;
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 1fe335a..e02f12d 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -1305,8 +1305,10 @@ void WebContentsImpl::CreateNewWindow(
     int instance_id = GetBrowserPluginGuestManager()->get_next_instance_id();
     WebContentsImpl* new_contents_impl =
         static_cast<WebContentsImpl*>(new_contents);
-    BrowserPluginGuest::CreateWithOpener(instance_id, new_contents_impl,
-        GetBrowserPluginGuest(), !!new_contents_impl->opener());
+    BrowserPluginGuest::CreateWithOpener(instance_id,
+                                         new_contents_impl->opener() != NULL,
+                                         new_contents_impl,
+                                         GetBrowserPluginGuest());
   }
   if (params.disposition == NEW_BACKGROUND_TAB)
     create_params.initially_hidden = true;