Enables hot-tracking for overflow extension buttons in the app menu

ui/views/controls/menu/menu_scroll_view_container.cc

Index: ui/views/controls/menu/menu_scroll_view_container.cc
diff --git a/ui/views/controls/menu/menu_scroll_view_container.cc b/ui/views/controls/menu/menu_scroll_view_container.cc
index 05c8caedb5505b4ac7c596195ec5d4aa02a4f7f0..09f8ed6234af805c1aba8f01f656c03678939db2 100644
--- a/ui/views/controls/menu/menu_scroll_view_container.cc
+++ b/ui/views/controls/menu/menu_scroll_view_container.cc
@@ -75,8 +75,7 @@ class MenuScrollButton : public View {
 
     // The background.
     gfx::Rect item_bounds(0, 0, width(), height());
-    NativeTheme::ExtraParams extra;
-    extra.menu_item.is_selected = false;
+    NativeTheme::ExtraParams extra = NativeTheme::ExtraParams();

[Peter Kasting, 2016/02/06 02:51:13]

Why is explicitly calling the null constructor needed?

[varkha, 2016/02/16 19:59:44]

To make sure all members are zero-initialized (NativeTheme::ExtraParams doesn't
have a declared constructor so the initial values are not defined).

[Peter Kasting, 2016/02/16 21:36:14]

I thought the standard said that for a union without a user-defined default
constructor, value initialization is zero initialization, and zero
initialization zero-initializes the first member of the union and then zeroes
the rest of the memory (padding) in the union.

I've never seen anyone need to explicitly call a default constructor like this. 
Its necessity would really surprise me.  Can you find some language (or
demonstrate via a test program output) that falsifies my understanding above?

[varkha, 2016/02/17 17:52:00]

Yes, I have tried this without this line and the corner radius was random. This
is because there is no initializer / constructor at all. C++ allows the first
member of a union to be initialized [1]. In our case the union is as initialized
as its first structure which does not have a constructor so this [2] applies and
thus there is no default value for any of the union members.

So I think that when there is some, even a partial initializer, the rest would
get zero-initialized but since we have none at all the initial values are not
defined.

Note that I am using a zero-init syntax (similar to "int var = int();"), not a
default constructor (which we do not have in code and maybe we should have in
this case). That is what is ensuring zero values [3].

As for the tracing output:
If I omit "= ui::NativeTheme::ExtraParams()" in line 220 of app_menu.cc I get
this output from the following added line:
221: LOG(ERROR) << __FUNCTION__ << " radius:" <<
params.menu_background.corner_radius;

[24068:24068:0217/121146:ERROR:app_menu.cc(221)] DrawBackground radius:24
[24068:24068:0217/121146:ERROR:app_menu.cc(221)] DrawBackground
radius:-761010977
[24068:24068:0217/121146:ERROR:app_menu.cc(221)] DrawBackground radius:410410428
[24068:24068:0217/121146:ERROR:app_menu.cc(221)] DrawBackground radius:24
[24068:24068:0217/121146:ERROR:app_menu.cc(221)] DrawBackground radius:24
[24068:24068:0217/121146:ERROR:app_menu.cc(221)] DrawBackground
radius:-761010977
[24068:24068:0217/121146:ERROR:app_menu.cc(221)] DrawBackground radius:24

If I have it, I get this:
[24721:24721:0217/121300:ERROR:app_menu.cc(221)] DrawBackground radius:0
[24721:24721:0217/121300:ERROR:app_menu.cc(221)] DrawBackground radius:0
[24721:24721:0217/121300:ERROR:app_menu.cc(221)] DrawBackground radius:0
[24721:24721:0217/121301:ERROR:app_menu.cc(221)] DrawBackground radius:0
[24721:24721:0217/121302:ERROR:app_menu.cc(221)] DrawBackground radius:0
[24721:24721:0217/121302:ERROR:app_menu.cc(221)] DrawBackground radius:0


[1] - "The default initializer for a union with static storage is the default
for the first component. A union with automatic storage has no default
initialization."
https://www-01.ibm.com/support/knowledgecenter/SSLTBW_2.1.0/com.ibm.zos.v2r1....

[2] - "If a structure variable does not have an initializer, the initial values
of the structure members depend on the storage class associated with the
structure variable:
If a structure variable has static storage, its members are implicitly
initialized to zero of the appropriate type.
If a structure variable has automatic storage, its members have no default
initialization.
If a structure variable is partially initialized, all the uninitialized
structure members are implicitly initialized to zero no matter what the storage
class of the structure variable is."

[3] - http://en.cppreference.com/w/cpp/language/zero_initialization

[Peter Kasting, 2016/02/17 22:12:04]

That language in your footnotes is exactly what I was missing.  Thanks!

I think the safest thing to do here is to add some sort of default constructor
to the union so that no one else can run into this issue in the future.  How's
that sound?

[varkha, 2016/02/17 23:15:57]

I'll do that.

[varkha, 2016/02/18 02:24:01]

Done.

     GetNativeTheme()->Paint(canvas->sk_canvas(),
                             NativeTheme::kMenuItemBackground,
                             NativeTheme::kNormal, item_bounds, extra);