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

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

 // Copyright 2015 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 "chrome/browser/ui/webui/media_router/media_router_webui_message_handler.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 46 matching lines @@
 const char kReportTimeToClickSink[] = "reportTimeToClickSink";
 const char kReportTimeToInitialActionClose[] = "reportTimeToInitialActionClose";
 const char kOnInitialDataReceived[] = "onInitialDataReceived";
 
 // JS function names.
 const char kSetInitialData[] = "media_router.ui.setInitialData";
 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);
 
     const MediaSink& sink = sink_with_cast_modes.sink;
     sink_val->SetString("id", sink.id());
     sink_val->SetString("name", sink.name());
     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(
     const MediaRoute& route, bool canJoin, const std::string& extension_id) {
   scoped_ptr<base::DictionaryValue> dictionary(new base::DictionaryValue);
   dictionary->SetString("id", route.media_route_id());
   dictionary->SetString("sinkId", route.media_sink_id());
   dictionary->SetString("description", route.description());
   dictionary->SetBoolean("isLocal", route.is_local());
   dictionary->SetBoolean("canJoin", canJoin);
@@ skipping 93 matching lines @@
       media_router_ui_(media_router_ui) {
   DCHECK(media_router_ui_);
 }
 
 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(
     const std::vector<MediaRoute>& routes,
     const std::vector<MediaRoute::Id>& joinable_route_ids) {
   scoped_ptr<base::ListValue> routes_val(RoutesToValue(routes,
       joinable_route_ids,
       media_router_ui_->GetRouteProviderExtensionId()));
   web_ui()->CallJavascriptFunction(kSetRouteList, *routes_val);
 }
@@ skipping 113 matching lines @@
 void MediaRouterWebUIMessageHandler::OnRequestInitialData(
     const base::ListValue* args) {
   DVLOG(1) << "OnRequestInitialData";
   media_router_ui_->OnUIInitiallyLoaded();
   base::DictionaryValue initial_data;
 
   // "No Cast devices found?" Chromecast help center page.
   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(),
       media_router_ui_->GetRouteProviderExtensionId()));
   initial_data.Set("routes", routes.release());
 
   const std::set<MediaCastMode> cast_modes = media_router_ui_->cast_modes();
   scoped_ptr<base::ListValue> cast_modes_list(
       CastModesToValue(cast_modes,
                        media_router_ui_->GetPresentationRequestSourceName()));
@@ skipping 50 matching lines @@
           sink_id, static_cast<MediaCastMode>(cast_mode_num))) {
     // TODO(imcheng): Need to add an issue if failed to initiate a CreateRoute
     // request.
     DVLOG(1) << "Error initiating route request.";
   }
 }
 
 void MediaRouterWebUIMessageHandler::OnAcknowledgeFirstRunFlow(
     const base::ListValue* args) {
   DVLOG(1) << "OnAcknowledgeFirstRunFlow";
   Profile::FromWebUI(web_ui())->GetPrefs()->SetBoolean(

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

nit: extract PrefService* into a local variable.

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

I'll get to this if I can.  Did these lines show up as being changed by me?

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

No these lines are not affected by your change.  Just thought it is a small
change that can be included since you are modifying this file anyway :)

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

Done.

       prefs::kMediaRouterFirstRunFlowAcknowledged, true);
 
 #if defined(GOOGLE_CHROME_BUILD)
   bool enabled_cloud_services = false;
   // Do not set the relevant cloud services prefs if the user was not shown
   // the cloud services prompt.
   if (!args->GetBoolean(0, &enabled_cloud_services)) {
     DVLOG(1) << "User was not shown the enable cloud services prompt.";
     return;
   }
@@ skipping 289 matching lines @@
                                  first_run_flow_acknowledged);
   first_run_flow_data.SetBoolean("showFirstRunFlowCloudPref", show_cloud_pref);
   web_ui()->CallJavascriptFunction(kSetFirstRunFlowData, first_run_flow_data);
 }
 
 void MediaRouterWebUIMessageHandler::SetWebUIForTest(content::WebUI* web_ui) {
   set_web_ui(web_ui);
 }
 
 }  // namespace media_router