Add update menu item and app menu icon badge

chrome/android/java/res/layout-sw600dp/toolbar.xml

Index: chrome/android/java/res/layout-sw600dp/toolbar.xml
diff --git a/chrome/android/java/res/layout-sw600dp/toolbar.xml b/chrome/android/java/res/layout-sw600dp/toolbar.xml
index a60efe7912376f9cf04ed60af88d7e5a6d18c984..ecdbb53f6e2adef4f9aa24ca69ab82f154aae284 100644
--- a/chrome/android/java/res/layout-sw600dp/toolbar.xml
+++ b/chrome/android/java/res/layout-sw600dp/toolbar.xml
@@ -55,12 +55,27 @@
             style="@style/ToolbarButton"
             android:contentDescription="@string/accessibility_toolbar_btn_tabswitcher_toggle"
             android:visibility="gone" />
-        <org.chromium.chrome.browser.widget.TintedImageButton
-            android:id="@+id/menu_button"
-            style="@style/ToolbarButton"
-            android:src="@drawable/btn_menu"
-            android:contentDescription="@string/accessibility_toolbar_btn_menu"
-            android:layout_width="43dp"
-            android:paddingEnd="3.5dp" />
+        <FrameLayout

[gone, 2015/12/08 22:24:23]

1) Does it make sense to pull this FrameLayout + children thing out into its own
Layout file, then include it from the three locations I see so far?

2) nit: could you add some newlines to break up this monolithic block of stuff? 
The attributes are indented at the same level as the children and makes it hard
to parse.

[Theresa, 2015/12/10 03:53:17]

1. All of the menu_buttons have different layout_width and padding so extracting
and the FrameLayout for reuse between both toolbar.xml files and
location_bar.xml will be difficult, but I'm not totally opposed to doing that. I
did extract some of the menu_badge style into a styles.xml.

Another idea, suggested by newt@, is to make a new class that extends
TintedImageButton, override the onDraw method and draw the badge in the correct
spot (if it should be shown).

I'm most inclined to leave the code how it is since the badge is experimental
(and less work is better), but can easily be talked into changing it.

2. done (are there too many now?)

[gone, 2015/12/10 21:45:07]

Looks good.  If this becomes a permanent thing I guess we could revisit.

[Theresa, 2015/12/11 19:44:46]

Acknowledged.

+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:id="@+id/menu_btn_wrapper" >

[gone, 2015/12/08 22:24:23]

nit: menu_button_wrapper?  I think the btn_ designation is just for filenames,
and the IDs above use forward_button / btn_forward as well.

[Theresa, 2015/12/10 03:53:17]

Done.

+            <org.chromium.chrome.browser.widget.TintedImageButton
+                android:id="@+id/menu_button"
+                style="@style/ToolbarButton"
+                android:src="@drawable/btn_menu"
+                android:contentDescription="@string/accessibility_toolbar_btn_menu"
+                android:layout_width="43dp"
+                android:paddingEnd="3.5dp" />
+            <ImageView
+                android:id="@+id/menu_badge"
+                android:src="@drawable/badge_update"
+                android:layout_height="@dimen/menu_badge_size"
+                android:layout_width="@dimen/menu_badge_size"
+                android:layout_marginBottom="14dp"
+                android:layout_marginEnd="9.5dp"
+                android:layout_gravity="end|bottom"
+                android:contentDescription="@string/accessibility_toolbar_update_badge"

[gone, 2015/12/08 22:24:23]

I don't know if it makes sense to apply the accessibility description to the
tiny badge; does it make sense to apply a common accessibility string to the
FrameLayout instead?  One that changes depending on whether an update is
available?

[Theresa, 2015/12/10 03:53:17]

I don't think views inherit contentDescriptions from their parent (if I move the
contentDescription from the TintedImageButton to the FrameLayout I get "Button
28, unlabeled"). I could add code to set it on the menu_button, but I don't
think that's necessary.

I mostly added a content description to the badge because lint complained that I
didn't have one. I don't think the accessibility string needs to change when the
menu is badged, so I made the badge non-focusable, labeled it as not important
for accessibility and set the contentDescription to null. A tap on the menu
button reads "More options button", which is accurate (and concise) whether a
badge is present or not.

[gone, 2015/12/10 21:45:07]

You might be able to apply a @null contentDescription, or define the ImageView
as being unimportant for accessibility:
http://developer.android.com/reference/android/view/View.html#attr_android:im...

[Theresa, 2015/12/11 19:44:46]

I set the contentDescription to null and defined it as being unimportant for
accessibility (in styles.xml).

+                android:visibility="gone" />
+        </FrameLayout>
     </LinearLayout>
 </org.chromium.chrome.browser.toolbar.ToolbarTablet>