[Media Router] Show user email in header if cloud sink is present.

chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc

Index: chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc
diff --git a/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc b/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc
index 7fa7f2a4e44ad03d96ae7f4ceb22d1e3bc7331e8..0d5483833d2baf85d4e2f06722bb503746b7eac4 100644
--- a/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc
+++ b/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc
@@ -64,15 +64,32 @@ const char kOnCreateRouteResponseReceived[] =
     "media_router.ui.onCreateRouteResponseReceived";
 const char kSetFirstRunFlowData[] = "media_router.ui.setFirstRunFlowData";
 const char kSetIssue[] = "media_router.ui.setIssue";
-const char kSetSinkList[] = "media_router.ui.setSinkList";
+const char kSetSinkListAndIdentity[] = "media_router.ui.setSinkListAndIdentity";
 const char kSetRouteList[] = "media_router.ui.setRouteList";
 const char kSetCastModeList[] = "media_router.ui.setCastModeList";
 const char kUpdateMaxHeight[] = "media_router.ui.updateMaxHeight";
 const char kWindowOpen[] = "window.open";
 
-scoped_ptr<base::ListValue> SinksToValue(
-    const std::vector<MediaSinkWithCastModes>& sinks) {
-  scoped_ptr<base::ListValue> value(new base::ListValue);
+scoped_ptr<base::DictionaryValue> SinksAndIdentityToValue(
+    const std::vector<MediaSinkWithCastModes>& sinks,
+    Profile* profile) {
+
+  scoped_ptr<base::DictionaryValue> sink_list_and_identity(
+      new base::DictionaryValue);
+
+#if defined(GOOGLE_CHROME_BUILD)
+  SigninManagerBase* signin_manager =
+      SigninManagerFactory::GetForProfile(profile);
+  std::string user_domain =
+      signin_manager->GetAuthenticatedAccountInfo().hosted_domain;

[imcheng, 2016/02/17 00:51:08]

the documentation says GetAuthenticatedAccountInfo() returns an empty struct if
user is not signed in. Is that case being handled?

[imcheng, 2016/02/17 00:51:08]

nit: store the result of signin_manager->GetAuthenticatedAccountInfo() in a
local variable to avoid looking up twice.

[amp, 2016/02/17 02:11:37]

Done.

[amp, 2016/02/17 02:11:37]

Done.  It should now, I'll make sure to have tests that cover this case (and
I'll try to verify what happens on my local builds).

+  sink_list_and_identity->SetString("userEmail",
+      signin_manager->GetAuthenticatedAccountInfo().email);
+  sink_list_and_identity->SetString("userDomain", user_domain);
+#endif  // defined(GOOGLE_CHROME_BUILD)
+
+  scoped_ptr<base::ListValue> sinks_val(new base::ListValue);
+  MediaRouterWebUIMessageHandler::IdentityVisibility identityVisibility =
+      MediaRouterWebUIMessageHandler::IdentityVisibility::HIDDEN;
 
   for (const MediaSinkWithCastModes& sink_with_cast_modes : sinks) {
     scoped_ptr<base::DictionaryValue> sink_val(new base::DictionaryValue);
@@ -83,18 +100,43 @@ scoped_ptr<base::ListValue> SinksToValue(
     sink_val->SetInteger("iconType", sink.icon_type());
     if (!sink.description().empty())
       sink_val->SetString("description", sink.description());
-    if (!sink.domain().empty())
-      sink_val->SetString("domain", sink.domain());
+#if defined(GOOGLE_CHROME_BUILD)

[imcheng, 2016/02/17 00:51:08]

Personally I think the logic to determine whether to show email and domain
should be in JS. But I understand the need to guard this with
GOOGLE_CHROME_BUILD. Though I wonder if there's a clean way for the UI to
implement this logic without having to use GOOGLE_CHROME_BUILD. What you have
here is fine though.

[amp, 2016/02/17 02:11:37]

With the change to the check on the account info struct above I can just have
this check that |user_domain| is not empty and remove the chrome build checks.

+    if (!sink.domain().empty()) {
+      std::string domain = sink.domain();
+      // Convert default domains to user domain
+      if (sink.domain() == "default")
+        domain = user_domain;
+
+      sink_val->SetString("domain", domain);
+
+      // Don't reset visibility to SHOW_EMAIL if it was already SHOW_DOMAIN
+      if (identityVisibility ==
+          MediaRouterWebUIMessageHandler::IdentityVisibility::HIDDEN)

[imcheng, 2016/02/17 00:51:08]

nit: need some braces for these if statements.

[amp, 2016/02/17 02:11:37]

With the removal of the visibility enum's these become simple boolean
assignments.  I'm not sure exactly what the style guidelines are, but that seems
to be allowed in other places in this file.

[imcheng, 2016/02/18 18:58:54]

I find adding braces help readability for single statements that span more than
1 line.

[amp, 2016/02/20 02:00:55]

Acknowledged.

+        identityVisibility =
+            MediaRouterWebUIMessageHandler::IdentityVisibility::SHOW_EMAIL;
+      if (domain != user_domain)
+        identityVisibility =
+            MediaRouterWebUIMessageHandler::IdentityVisibility::SHOW_DOMAIN;
+    }
+#endif  // defined(GOOGLE_CHROME_BUILD)
 
     int cast_mode_bits = 0;
     for (MediaCastMode cast_mode : sink_with_cast_modes.cast_modes)
       cast_mode_bits |= cast_mode;
 
     sink_val->SetInteger("castModes", cast_mode_bits);
-    value->Append(sink_val.release());
+    sinks_val->Append(sink_val.release());
   }
 
-  return value;
+  sink_list_and_identity->Set("sinks", sinks_val.release());
+
+  sink_list_and_identity->SetBoolean("showEmail",
+      identityVisibility !=
+          MediaRouterWebUIMessageHandler::IdentityVisibility::HIDDEN);
+  sink_list_and_identity->SetBoolean("showDomain",
+      identityVisibility ==
+          MediaRouterWebUIMessageHandler::IdentityVisibility::SHOW_DOMAIN);
+  return sink_list_and_identity;
 }
 
 scoped_ptr<base::DictionaryValue> RouteToValue(
@@ -208,8 +250,11 @@ MediaRouterWebUIMessageHandler::~MediaRouterWebUIMessageHandler() {
 void MediaRouterWebUIMessageHandler::UpdateSinks(
     const std::vector<MediaSinkWithCastModes>& sinks) {
   DVLOG(2) << "UpdateSinks";
-  scoped_ptr<base::ListValue> sinks_val(SinksToValue(sinks));
-  web_ui()->CallJavascriptFunction(kSetSinkList, *sinks_val);
+  Profile* profile = Profile::FromWebUI(web_ui());
+  scoped_ptr<base::DictionaryValue> sinks_and_identity_val(
+      SinksAndIdentityToValue(sinks, profile));
+  web_ui()->CallJavascriptFunction(
+      kSetSinkListAndIdentity, *sinks_and_identity_val);
 }
 
 void MediaRouterWebUIMessageHandler::UpdateRoutes(
@@ -343,8 +388,10 @@ void MediaRouterWebUIMessageHandler::OnRequestInitialData(
   initial_data.SetString("deviceMissingUrl",
       base::StringPrintf(kHelpPageUrlPrefix, 3249268));
 
-  scoped_ptr<base::ListValue> sinks(SinksToValue(media_router_ui_->sinks()));
-  initial_data.Set("sinks", sinks.release());
+  Profile* profile = Profile::FromWebUI(web_ui());
+  scoped_ptr<base::DictionaryValue> sinks_and_identity(SinksAndIdentityToValue(
+      media_router_ui_->sinks(), profile));

[imcheng, 2016/02/17 00:51:08]

nit: inline profile.

[amp, 2016/02/17 02:11:37]

Done.

+  initial_data.Set("sinksAndIdentity", sinks_and_identity.release());
 
   scoped_ptr<base::ListValue> routes(RoutesToValue(media_router_ui_->routes(),
       media_router_ui_->joinable_route_ids(),