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

ui/views/controls/menu/menu_scroll_view_container.cc

 // 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.
 
 #include "ui/views/controls/menu/menu_scroll_view_container.h"
 
 #include "base/macros.h"
 #include "third_party/skia/include/core/SkPaint.h"
 #include "third_party/skia/include/core/SkPath.h"
 #include "ui/accessibility/ax_view_state.h"
@@ skipping 57 matching lines @@
 
   int OnPerformDrop(const ui::DropTargetEvent& event) override {
     return ui::DragDropTypes::DRAG_NONE;
   }
 
   void OnPaint(gfx::Canvas* canvas) override {
     const MenuConfig& config = MenuConfig::instance();
 
     // 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);
 
     // Then the arrow.
     int x = width() / 2;
     int y = (height() - config.scroll_arrow_height) / 2;
 
     int x_left = x - config.scroll_arrow_height;
     int x_right = x + config.scroll_arrow_height;
@@ skipping 232 matching lines @@
     case MENU_ANCHOR_BUBBLE_ABOVE:
       return BubbleBorder::BOTTOM_CENTER;
     case MENU_ANCHOR_BUBBLE_BELOW:
       return BubbleBorder::TOP_CENTER;
     default:
       return BubbleBorder::NONE;
   }
 }
 
 }  // namespace views