Browser Plugin: Implement autosize (Embedder-side code)

Browser Plugin: Implement autosize (Embedder-side code)

Snippet from bug:

"When using <webview> to embed widgets that change in size (e.g. the +1 button has various hover bubbles that can appear), it can be tedious (and inefficient) to make the contents monitor their size and inform the embedder that the <webview> needs to be resize."

This patch doesn't fully implement this feature, but in the interest of smaller, easier to review patches, this can stand alone.

This patch implements autoSize, minWidth, minHeight, maxWidth, maxHeight attributes on BrowserPlugin.

If these attributes are set before first navigation, they are propagated to the browser process on
BrowserPluginHostMsg_CreateGuest. 

If these attributes or updated after navigation, they are propagated via BrowserPluginHostMsg_SetAutoSize.

BUG=157620
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=165817

[Fady Samuel, 2012-11-02 00:22:39]

Hi John (Charlie in CC),

This is the beginnings of my implementation of autosize. This feature was
requested by Mihai to better support embedding widgets in <webview>.

Thanks,

Fady

[jam, 2012-11-02 19:11:10]

lgtm with nits

https://codereview.chromium.org/11361052/diff/1/content/browser/browser_plugi...
File content/browser/browser_plugin/browser_plugin_guest.h (right):

https://codereview.chromium.org/11361052/diff/1/content/browser/browser_plugi...
content/browser/browser_plugin/browser_plugin_guest.h:266: int minwidth_;
ditto

https://codereview.chromium.org/11361052/diff/1/content/common/browser_plugin...
File content/common/browser_plugin_messages.h (right):

https://codereview.chromium.org/11361052/diff/1/content/common/browser_plugin...
content/common/browser_plugin_messages.h:37: // This message is sent to the
browser process to enable or disable autosize
nit: put this comment on the IPC_MESSAGE, not the struct

https://codereview.chromium.org/11361052/diff/1/content/common/browser_plugin...
content/common/browser_plugin_messages.h:39:
IPC_STRUCT_BEGIN(BrowserPluginHostMsg_AutoSize_Params)
nit: structs go at the top of the file and not by the messages (i.e. at line 30)

https://codereview.chromium.org/11361052/diff/1/content/common/browser_plugin...
content/common/browser_plugin_messages.h:41: IPC_STRUCT_MEMBER(int, maxheight)
max_height

https://codereview.chromium.org/11361052/diff/1/content/renderer/browser_plug...
File content/renderer/browser_plugin/browser_plugin.cc (right):

https://codereview.chromium.org/11361052/diff/1/content/renderer/browser_plug...
content/renderer/browser_plugin/browser_plugin.cc:202: int
BrowserPlugin::GetMaxHeightAttribute() const {
nit: all these getters should jus be unix_hacker style methods in the header,
i.e. int max_height_attribute() const { return max_height_; }

https://codereview.chromium.org/11361052/diff/1/content/renderer/browser_plug...
File content/renderer/browser_plugin/browser_plugin.h (right):

https://codereview.chromium.org/11361052/diff/1/content/renderer/browser_plug...
content/renderer/browser_plugin/browser_plugin.h:255: bool autosize_;
nit: auto_size_
also below, max_height_, max_width_ etc...

per convention

https://codereview.chromium.org/11361052/diff/1/content/renderer/browser_plug...
File content/renderer/browser_plugin/browser_plugin_bindings.cc (right):

https://codereview.chromium.org/11361052/diff/1/content/renderer/browser_plug...
content/renderer/browser_plugin/browser_plugin_bindings.cc:297: int minheight =
Int32FromNPVariant(*variant);
ditto

[Fady Samuel, 2012-11-02 20:32:23]

Fixed nits. CQ'ing now.

http://codereview.chromium.org/11361052/diff/1/content/browser/browser_plugin...
File content/browser/browser_plugin/browser_plugin_guest.h (right):

http://codereview.chromium.org/11361052/diff/1/content/browser/browser_plugin...
content/browser/browser_plugin/browser_plugin_guest.h:266: int minwidth_;
On 2012/11/02 19:11:10, John Abd-El-Malek wrote:
> ditto

Done.

http://codereview.chromium.org/11361052/diff/1/content/common/browser_plugin_...
File content/common/browser_plugin_messages.h (right):

http://codereview.chromium.org/11361052/diff/1/content/common/browser_plugin_...
content/common/browser_plugin_messages.h:37: // This message is sent to the
browser process to enable or disable autosize
On 2012/11/02 19:11:10, John Abd-El-Malek wrote:
> nit: put this comment on the IPC_MESSAGE, not the struct

Done.

http://codereview.chromium.org/11361052/diff/1/content/common/browser_plugin_...
content/common/browser_plugin_messages.h:39:
IPC_STRUCT_BEGIN(BrowserPluginHostMsg_AutoSize_Params)
On 2012/11/02 19:11:10, John Abd-El-Malek wrote:
> nit: structs go at the top of the file and not by the messages (i.e. at line
30)

Done.

http://codereview.chromium.org/11361052/diff/1/content/common/browser_plugin_...
content/common/browser_plugin_messages.h:41: IPC_STRUCT_MEMBER(int, maxheight)
On 2012/11/02 19:11:10, John Abd-El-Malek wrote:
> max_height

Done.

http://codereview.chromium.org/11361052/diff/1/content/renderer/browser_plugi...
File content/renderer/browser_plugin/browser_plugin.cc (right):

http://codereview.chromium.org/11361052/diff/1/content/renderer/browser_plugi...
content/renderer/browser_plugin/browser_plugin.cc:202: int
BrowserPlugin::GetMaxHeightAttribute() const {
On 2012/11/02 19:11:10, John Abd-El-Malek wrote:
> nit: all these getters should jus be unix_hacker style methods in the header,
> i.e. int max_height_attribute() const { return max_height_; }

Done.

http://codereview.chromium.org/11361052/diff/1/content/renderer/browser_plugi...
File content/renderer/browser_plugin/browser_plugin.h (right):

http://codereview.chromium.org/11361052/diff/1/content/renderer/browser_plugi...
content/renderer/browser_plugin/browser_plugin.h:255: bool autosize_;
On 2012/11/02 19:11:10, John Abd-El-Malek wrote:
> nit: auto_size_
> also below, max_height_, max_width_ etc...
> 
> per convention

Done.

http://codereview.chromium.org/11361052/diff/1/content/renderer/browser_plugi...
File content/renderer/browser_plugin/browser_plugin_bindings.cc (right):

http://codereview.chromium.org/11361052/diff/1/content/renderer/browser_plugi...
content/renderer/browser_plugin/browser_plugin_bindings.cc:297: int minheight =
Int32FromNPVariant(*variant);
On 2012/11/02 19:11:10, John Abd-El-Malek wrote:
> ditto

Done.

[commit-bot: I haz the power, 2012-11-02 20:34:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fsamuel@chromium.org/11361052/5002

[commit-bot: I haz the power, 2012-11-02 20:34:56]

Failed to apply patch for content/common/browser_plugin_messages.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/common/browser_plugin_messages.h
  Hunk #4 FAILED at 269.
  1 out of 4 hunks FAILED -- saving rejects to file
content/common/browser_plugin_messages.h.rej

Patch:       content/common/browser_plugin_messages.h
Index: content/common/browser_plugin_messages.h
diff --git a/content/common/browser_plugin_messages.h
b/content/common/browser_plugin_messages.h
index
ea4d847559a79409f6bf0331cce79f92ad82a3a3..ab57bf80edc84bbf06827c5a21ddf6949cf92173
100644
--- a/content/common/browser_plugin_messages.h
+++ b/content/common/browser_plugin_messages.h
@@ -29,20 +29,114 @@
 
 IPC_ENUM_TRAITS(WebKit::WebDragStatus)
 
+IPC_STRUCT_BEGIN(BrowserPluginHostMsg_AutoSize_Params)
+  IPC_STRUCT_MEMBER(int, enable)
+  IPC_STRUCT_MEMBER(int, max_height)
+  IPC_STRUCT_MEMBER(int, max_width)
+  IPC_STRUCT_MEMBER(int, min_height)
+  IPC_STRUCT_MEMBER(int, min_width)
+IPC_STRUCT_END()
+
+IPC_STRUCT_BEGIN(BrowserPluginHostMsg_CreateGuest_Params)
+  IPC_STRUCT_MEMBER(std::string, storage_partition_id)
+  IPC_STRUCT_MEMBER(bool, persist_storage)
+  IPC_STRUCT_MEMBER(bool, focused)
+  IPC_STRUCT_MEMBER(bool, visible)
+  IPC_STRUCT_MEMBER(BrowserPluginHostMsg_AutoSize_Params, auto_size)
+IPC_STRUCT_END()
+
+IPC_STRUCT_BEGIN(BrowserPluginHostMsg_ResizeGuest_Params)
+  // An identifier to the new buffer to use to transport damage to the embedder
+  // renderer process.
+  IPC_STRUCT_MEMBER(TransportDIB::Id, damage_buffer_id)
+#if defined(OS_MACOSX)
+  // On OSX, a handle to the new buffer is used to map the transport dib since
+  // we don't let browser manage the dib.
+  IPC_STRUCT_MEMBER(TransportDIB::Handle, damage_buffer_handle)
+#endif
+#if defined(OS_WIN)
+  // The size of the damage buffer because this information is not available
+  // on Windows.
+  IPC_STRUCT_MEMBER(int, damage_buffer_size)
+#endif
+  // The new width of the plugin container.
+  IPC_STRUCT_MEMBER(int, width)
+  // The new height of the plugin container.
+  IPC_STRUCT_MEMBER(int, height)
+  // Indicates whether the embedder is currently waiting on a ACK from the
+  // guest for a previous resize request.
+  IPC_STRUCT_MEMBER(bool, resize_pending)
+  // Indicates the scale factor of the embedder WebView.
+  IPC_STRUCT_MEMBER(float, scale_factor)
+IPC_STRUCT_END()
+
+IPC_STRUCT_BEGIN(BrowserPluginMsg_LoadCommit_Params)
+  // The current URL of the guest.
+  IPC_STRUCT_MEMBER(GURL, url)
+  // Indicates whether the navigation was on the top-level frame.
+  IPC_STRUCT_MEMBER(bool, is_top_level)
+  // Chrome's process ID for the guest.
+  IPC_STRUCT_MEMBER(int, process_id)
+  // The index of the current navigation entry after this navigation was
+  // committed.
+  IPC_STRUCT_MEMBER(int, current_entry_index)
+  // The number of navigation entries after this navigation was committed.
+  IPC_STRUCT_MEMBER(int, entry_count)
+IPC_STRUCT_END()
+
+IPC_STRUCT_BEGIN(BrowserPluginMsg_UpdateRect_Params)
+  // The position and size of the bitmap.
+  IPC_STRUCT_MEMBER(gfx::Rect, bitmap_rect)
+
+  // The scroll offset.  Only one of these can be non-zero, and if they are
+  // both zero, then it means there is no scrolling and the scroll_rect is
+  // ignored.
+  IPC_STRUCT_MEMBER(int, dx)
+  IPC_STRUCT_MEMBER(int, dy)
+
+  // The rectangular region to scroll.
+  IPC_STRUCT_MEMBER(gfx::Rect, scroll_rect)
+
+  // The scroll offset of the render view.
+  IPC_STRUCT_MEMBER(gfx::Point, scroll_offset)
+
+  // The regions of the bitmap (in view coords) that contain updated pixels.
+  // In the case of scrolling, this includes the scroll damage rect.
+  IPC_STRUCT_MEMBER(std::vector<gfx::Rect>, copy_rects)
+
+  // The size of the RenderView when this message was generated.  This is
+  // included so the host knows how large the view is from the perspective of
+  // the renderer process.  This is necessary in case a resize operation is in
+  // progress. If auto-resize is enabled, this should update the corresponding
+  // view size.
+  IPC_STRUCT_MEMBER(gfx::Size, view_size)
+
+  // All the above coordinates are in DIP. This is the scale factor needed
+  // to convert them to pixels.
+  IPC_STRUCT_MEMBER(float, scale_factor)
+
+  // Is this UpdateRect an ACK to a resize request?
+  IPC_STRUCT_MEMBER(bool, is_resize_ack)
+IPC_STRUCT_END()
+
 // Browser plugin messages
 
 //
-----------------------------------------------------------------------------
 // These messages are from the embedder to the browser process.
 
+// This message is sent to the browser process to enable or disable autosize
+// mode.
+IPC_MESSAGE_ROUTED2(BrowserPluginHostMsg_SetAutoSize,
+                    int /* instance_id */,
+                    BrowserPluginHostMsg_AutoSize_Params)
+
+
 // This message is sent to the browser process to create the browser plugin
 // embedder and helper. It is sent once prior to sending the first
 // BrowserPluginHostMsg_NavigateGuest message.
-IPC_MESSAGE_ROUTED5(BrowserPluginHostMsg_CreateGuest,
+IPC_MESSAGE_ROUTED2(BrowserPluginHostMsg_CreateGuest,
                     int /* instance_id */,
-                    std::string /* storage_partition_id */,
-                    bool /* persist_storage */,
-                    bool /* focused */,
-                    bool /* visible */)
+                    BrowserPluginHostMsg_CreateGuest_Params /* params */)
 
 // Tells the browser process to terminate the guest associated with the
 // browser plugin associated with the provided |instance_id|.
@@ -85,31 +179,6 @@ IPC_MESSAGE_ROUTED3(BrowserPluginHostMsg_UpdateRect_ACK,
                     int /* message_id */,
                     gfx::Size /* repaint_view_size */)
 
-IPC_STRUCT_BEGIN(BrowserPluginHostMsg_ResizeGuest_Params)
-  // An identifier to the new buffer to use to transport damage to the embedder
-  // renderer process.
-  IPC_STRUCT_MEMBER(TransportDIB::Id, damage_buffer_id)
-#if defined(OS_MACOSX)
-  // On OSX, a handle to the new buffer is used to map the transport dib since
-  // we don't let browser manage the dib.
-  IPC_STRUCT_MEMBER(TransportDIB::Handle, damage_buffer_handle)
-#endif
-#if defined(OS_WIN)
-  // The size of the damage buffer because this information is not available
-  // on Windows.
-  IPC_STRUCT_MEMBER(int, damage_buffer_size)
-#endif
-  // The new width of the plugin container.
-  IPC_STRUCT_MEMBER(int, width)
-  // The new height of the plugin container.
-  IPC_STRUCT_MEMBER(int, height)
-  // Indicates whether the embedder is currently waiting on a ACK from the
-  // guest for a previous resize request.
-  IPC_STRUCT_MEMBER(bool, resize_pending)
-  // Indicates the scale factor of the embedder WebView.
-  IPC_STRUCT_MEMBER(float, scale_factor)
-IPC_STRUCT_END()
-
 // A BrowserPlugin sends this to BrowserPluginEmbedder (browser process) when
it
 // wants to navigate to a given src URL. If a guest WebContents already exists,
 // it will navigate that WebContents. If not, it will create the WebContents,
@@ -182,20 +251,6 @@ IPC_MESSAGE_CONTROL4(BrowserPluginMsg_LoadRedirect,
                      GURL /* new_url */,
                      bool /* is_top_level */)
 
-IPC_STRUCT_BEGIN(BrowserPluginMsg_LoadCommit_Params)
-  // The current URL of the guest.
-  IPC_STRUCT_MEMBER(GURL, url)
-  // Indicates whether the navigation was on the top-level frame.
-  IPC_STRUCT_MEMBER(bool, is_top_level)
-  // Chrome's process ID for the guest.
-  IPC_STRUCT_MEMBER(int, process_id)
-  // The index of the current navigation entry after this navigation was
-  // committed.
-  IPC_STRUCT_MEMBER(int, current_entry_index)
-  // The number of navigation entries after this navigation was committed.
-  IPC_STRUCT_MEMBER(int, entry_count)
-IPC_STRUCT_END()
-
 // When the guest commits a navigation, the browser process informs
 // the embedder through the BrowserPluginMsg_DidCommit message.
 IPC_MESSAGE_CONTROL2(BrowserPluginMsg_LoadCommit,
@@ -214,40 +269,6 @@ IPC_MESSAGE_CONTROL3(BrowserPluginMsg_GuestGone,
                      int /* process_id */,
                      int /* This is really base::TerminationStatus */)
 
-IPC_STRUCT_BEGIN(BrowserPluginMsg_UpdateRect_Params)
-  // The position and size of the bitmap.
-  IPC_STRUCT_MEMBER(gfx::Rect, bitmap_rect)
-
-  // The scroll offset.  Only one of these can be non-zero, and if they are
-  // both zero, then it means there is no scrolling and the scroll_rect is
-  // ignored.
-  IPC_STRUCT_MEMBER(int, dx)
-  IPC_STRUCT_MEMBER(int, dy)
-
-  // The rectangular region to scroll.
-  IPC_STRUCT_MEMBER(gfx::Rect, scroll_rect)
-
-  // The scroll offset of the render view.
-  IPC_STRUCT_MEMBER(gfx::Point, scroll_offset)
-
-  // The regions of the bitmap (in view coords) that contain updated pixels.
-  // In the case of scrolling, this includes the scroll damage rect.
-  IPC_STRUCT_MEMBER(std::vector<gfx::Rect>, copy_rects)
-
-  // The size of the RenderView when this message was generated.  This is
-  // included so the host knows how large the view is from the perspective of
-  // the renderer process.  This is necessary in case a resize operation is in
-  // progress. If auto-resize is enabled, this should update the corresponding
-  // view size.
-  IPC_STRUCT_MEMBER(gfx::Size, view_size)
-
-  // All the above coordinates are in DIP. This is the scale factor needed
-  // to convert them to pixels.
-  IPC_STRUCT_MEMBER(float, scale_factor)
-
-  // Is this UpdateRect an ACK to a resize request?
-  IPC_STRUCT_MEMBER(bool, is_resize_ack)
-IPC_STRUCT_END()
 
 // When the user tabs to the end of the tab stops of a guest, the browser
 // process informs the embedder to tab out of the browser plugin.

[commit-bot: I haz the power, 2012-11-02 21:41:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fsamuel@chromium.org/11361052/4002

[commit-bot: I haz the power, 2012-11-02 22:40:30]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-11-02 22:58:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fsamuel@chromium.org/11361052/10021

[commit-bot: I haz the power, 2012-11-03 00:49:41]

Change committed as 165817