C++ readability review for lazyboy@

content/browser/browser_plugin/browser_plugin_guest.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.
 
 // A BrowserPluginGuest is the browser side of a browser <--> embedder
 // renderer channel. A BrowserPlugin (a WebPlugin) is on the embedder
 // renderer side of browser <--> embedder renderer communication.
 //
 // BrowserPluginGuest lives on the UI thread of the browser process. Any
 // messages about the guest render process that the embedder might be interested
@@ skipping 84 matching lines @@
   // embedder needs to be aware of |guest_site_instance| on the guest's
   // construction and so we pass it in here.
   static BrowserPluginGuest* Create(
       int instance_id,
       SiteInstance* guest_site_instance,
       WebContentsImpl* web_contents,
       scoped_ptr<base::DictionaryValue> extra_params);
 
   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.
   void EmbedderDestroyed();
 
   // Called when the embedder WebContents changes visibility.
   void EmbedderVisibilityChanged(bool visible);
 
   // Destroys the guest WebContents and all its associated state, including
   // this BrowserPluginGuest, and its new unattached windows.
   void Destroy();
 
   // Returns the identifier that uniquely identifies a browser plugin guest
   // 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_;
   }
 
   RenderWidgetHostView* GetEmbedderRenderWidgetHostView();
 
   bool focused() const { return focused_; }
   bool visible() const { return guest_visible_; }
   void clear_damage_buffer() { damage_buffer_.reset(); }
@@ skipping 95 matching lines @@
 
   gfx::Point GetScreenCoordinates(const gfx::Point& relative_position) const;
 
   // Helper to send messages to embedder. This methods fills the message with
   // the correct routing id.
   // Overridden in test implementation since we want to intercept certain
   // messages for testing.
   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
   // to an embedder implies that this guest's lifetime is no longer managed
   // by its opener, and it can begin loading resources. |extra_params| are
   // parameters passed into BrowserPlugin from JavaScript to be forwarded to
   // the content embedder.
   void Attach(WebContentsImpl* embedder_web_contents,
               BrowserPluginHostMsg_Attach_Params params,
               const base::DictionaryValue& extra_params);
@@ skipping 26 matching lines @@
   // Called when the drag started by this guest ends at an OS-level.
   void EndSystemDrag();
 
   // |this| takes ownership of |delegate|.
   void SetDelegate(BrowserPluginGuestDelegate* delegate);
 
   void RespondToPermissionRequest(int request_id,
                                   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;
 
   class DownloadRequest;
   class GeolocationRequest;
   class JavaScriptDialogRequest;
   // MediaRequest because of naming conflicts with MediaStreamRequest.
   class MediaRequest;
   class NewWindowRequest;
   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
   // provided |bridge_id|. It returns the request ID of the geolocation request.
   int RemoveBridgeID(int bridge_id);
 
   // Returns the |request_id| generated for the |request| provided.
   int RequestPermission(
       BrowserPluginPermissionType permission_type,
       scoped_refptr<BrowserPluginGuest::PermissionRequest> request,
       const base::DictionaryValue& request_info);
 
   // Creates a new guest window, and BrowserPluginGuest that is owned by this
   // BrowserPluginGuest.
   BrowserPluginGuest* CreateNewGuestWindow(const OpenURLParams& params);
 
   base::SharedMemory* damage_buffer() const { return damage_buffer_.get(); }
   const gfx::Size& damage_view_size() const { return damage_view_size_; }
   float damage_buffer_scale_factor() const {
     return damage_buffer_scale_factor_;
   }
   // Returns the damage buffer corresponding to the handle in resize |params|.
   base::SharedMemory* GetDamageBufferFromEmbedder(
       const BrowserPluginHostMsg_ResizeGuest_Params& params);
 
   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.
 
   void OnCompositorFrameACK(int instance_id,
                             int route_id,
                             uint32 output_surface_id,
                             int renderer_host_id,
                             const cc::CompositorFrameAck& ack);
 
   // Handles drag events from the embedder.
@@ skipping 131 matching lines @@
   bool focused_;
   bool mouse_locked_;
   bool pending_lock_request_;
   bool guest_visible_;
   bool embedder_visible_;
   std::string name_;
   bool auto_size_enabled_;
   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_;
   // A counter to generate a unique request id for a permission request.
   // We only need the ids to be unique for a given BrowserPluginGuest.
   int next_permission_request_id_;
 
   // A map to store relevant info for a request keyed by the request's id.
   typedef std::map<int, scoped_refptr<PermissionRequest> > RequestMap;
   RequestMap permission_request_map_;
@@ skipping 19 matching lines @@
   // These are parameters passed from JavaScript on attachment to the content
   // embedder.
   scoped_ptr<base::DictionaryValue> extra_attach_params_;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserPluginGuest);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_BROWSER_PLUGIN_BROWSER_PLUGIN_GUEST_H_