Add a row in the network tray to inform users to turn Bluetooth on to enable Tether.

ash/system/tray/tray_details_view.h

Index: ash/system/tray/tray_details_view.h
diff --git a/ash/system/tray/tray_details_view.h b/ash/system/tray/tray_details_view.h
index f046fcdcdeec65b9c5ef0b51d7c26a5ff1a25161..bc3656381ca333841f6cb838248ebac051074913 100644
--- a/ash/system/tray/tray_details_view.h
+++ b/ash/system/tray/tray_details_view.h
@@ -8,11 +8,13 @@
 #include <memory>
 
 #include "ash/ash_export.h"
+#include "ash/system/tray/actionable_view.h"
 #include "ash/system/tray/tray_constants.h"
 #include "ash/system/tray/view_click_listener.h"
 #include "base/macros.h"
 #include "base/timer/timer.h"
 #include "ui/views/controls/button/button.h"
+#include "ui/views/controls/button/label_button.h"
 #include "ui/views/view.h"
 
 namespace gfx {
@@ -54,23 +56,39 @@ class ASH_EXPORT TrayDetailsView : public views::View,
 
   SystemTrayItem* owner() { return owner_; }
 
- protected:
+  class InfoLabelDelegate;

[Kyle Horimoto, 2017/07/10 18:37:08]

We should still keep InfoLabel protected. There shouldn't be any need for other
classes to be aware of the implementation details of TrayDetailsView.

To rectify this:
(1) Keep InfoLabelDelegate public, but change its name (since the caller
shouldn't have to be aware of what an InfoLabel is). How about
TrayDetailClickHandler?
(2) Currently, OnLabelClicked() refers to a label as well. How about
OnTrayDetailClicked()?
(3) You pass an InfoLabel as a parameter to the delegate function, but the
client shouldn't need to be aware of the implementation of the label. Instead,
just pass the message ID of the item that was clicked. In your NetworkList
implementation, the only thing you use the InfoLabel for is to fetch its message
ID, so that should be sufficient.

[James Cook, 2017/07/11 23:14:15]

I chatted with Leslie over IM. I suggest a different approach, which is more
similar to some of our other system tray code.

tray_details_view.cc is pretty long (>500 lines) and this CL makes it longer. I
would prefer to pull InfoLabel out into its own file. I would call it
TrayInfoLabel or similar and document that it is related to TrayDetailsView.
Since it would become public it would be easier to add tests for it.

I would do the delegate like:

class TrayInfoLabel : public <stuff> {
 public:
  class Delegate {
    ...stuff...
  }
  TrayInfoLabel(Delegate* delegate, ...);
}

[lesliewatkins, 2017/07/12 21:49:50]

Done.

+
   // A view containing only a label, which is to be inserted as a non-targetable
   // row within a system menu detailed view (e.g., the "Scanning for devices..."
   // message that can appear at the top of the Bluetooth detailed view).

[Kyle Horimoto, 2017/07/10 18:37:08]

nit: Now that this is clickable, add some description to the class regarding the
click handler delegate.

[lesliewatkins, 2017/07/12 21:49:50]

Done.

-  class InfoLabel : public View {
+  class InfoLabel : public ActionableView {
    public:
-    explicit InfoLabel(int message_id);
-    ~InfoLabel() override;

[Kyle Horimoto, 2017/07/10 18:37:08]

Don't remove the destructor.

+    explicit InfoLabel(int message_id, InfoLabelDelegate* delegate);

[James Cook, 2017/07/10 18:43:03]

no explicit

nit: Usually the delegate* is the first parameter.

Document that delegate can be null and what happens in that case.

Class still needs virtual destructor

[lesliewatkins, 2017/07/12 21:49:50]

Done.

+    void Update(int message_id);

[Kyle Horimoto, 2017/07/10 18:37:08]

nit: Previously, SetMessage() was pretty obvious in what it did (due to its
name), so a description wasn't necessary. Now, Update() is a bit more vaguer, so
please add a description.

[lesliewatkins, 2017/07/12 21:49:50]

Done.

+    const int& message_id();

[Kyle Horimoto, 2017/07/10 18:37:08]

You can get rid of this function after my explanation earlier.

[James Cook, 2017/07/10 18:43:03]

just return the int, there's no perf win for returning a reference to it. Also,
you can inline the code in the header. And the method should be const.

[lesliewatkins, 2017/07/12 21:49:50]

Acknowledged.

[lesliewatkins, 2017/07/12 21:49:50]

Done.

 
-    void SetMessage(int message_id);
+    // ActionableView:
+    bool PerformAction(const ui::Event& event) override;
+    void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
 
    private:
+    bool IsClickable();
+
     views::Label* const label_;
+    int message_id_;
+
+    InfoLabelDelegate* delegate_;
 
     DISALLOW_COPY_AND_ASSIGN(InfoLabel);
   };
 
+  // A delegate for handling actions when an InfoLabel is clicked.
+  class InfoLabelDelegate {

[James Cook, 2017/07/10 18:43:03]

optional: You could also declare this as an inner class of InfoLabel.

[lesliewatkins, 2017/07/12 21:49:50]

Acknowledged.

+   public:
+    virtual void OnLabelClicked(InfoLabel* label) = 0;
+  };

[James Cook, 2017/07/10 18:43:03]

needs virtual destructor

[lesliewatkins, 2017/07/12 21:49:50]

Done.

+
+ protected:
   // views::View:
   void Layout() override;
   int GetHeightForWidth(int width) const override;