[Media Router] Clean up issues related code.

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 <utility>
 
 #include "base/bind.h"
@@ skipping 168 matching lines @@
         "description", MediaCastModeToDescription(cast_mode, source_host));
     cast_mode_val->SetString("host", source_host);
     value->Append(std::move(cast_mode_val));
   }
 
   return value;
 }
 
 // Returns an Issue dictionary created from |issue| that can be used in WebUI.
 std::unique_ptr<base::DictionaryValue> IssueToValue(const Issue& issue) {
+  const IssueInfo& issue_info = issue.info();
   std::unique_ptr<base::DictionaryValue> dictionary(new base::DictionaryValue);
-  dictionary->SetString("id", issue.id());
-  dictionary->SetString("title", issue.title());
-  dictionary->SetString("message", issue.message());
-  dictionary->SetInteger("defaultActionType", issue.default_action().type());
-  if (!issue.secondary_actions().empty()) {
+  dictionary->SetInteger("id", issue.id());
+  dictionary->SetString("title", issue_info.title);
+  dictionary->SetString("message", issue_info.message);
+  dictionary->SetInteger("defaultActionType",
+                         static_cast<int>(issue_info.default_action));
+  if (!issue_info.secondary_actions.empty()) {

[msw, 2016/12/13 18:45:39]

tangential q: Are there ever instances where there are multiple secondary
actions? Should this DCHECK that there's only one, or should the vector just
become a single item?

[imcheng, 2016/12/13 19:22:47]

Our UI does not support showing multiple secondary actions, hence only the first
one is taken here. There aren't any Issues with >1 secondary actions (we also
own the code that creates the Issues). I can add a DCHECK here.

We will rewrite our UX in the future, and it would be a good time to consider
the following to simplify our data model: whether there needs to be a
distinction between default and secondary actions, and whether >2 actions are
needed (and if there should be an upper limit, how they should be shown if so).

     dictionary->SetInteger("secondaryActionType",
-                           issue.secondary_actions().begin()->type());
+                           static_cast<int>(issue_info.secondary_actions[0]));
   }
-  if (!issue.route_id().empty())
-    dictionary->SetString("routeId", issue.route_id());
-  dictionary->SetBoolean("isBlocking", issue.is_blocking());
-  if (issue.help_page_id() > 0)
-    dictionary->SetInteger("helpPageId", issue.help_page_id());
+  if (!issue_info.route_id.empty())
+    dictionary->SetString("routeId", issue_info.route_id);
+  dictionary->SetBoolean("isBlocking", issue_info.is_blocking);
+  if (issue_info.help_page_id > 0)
+    dictionary->SetInteger("helpPageId", issue_info.help_page_id);
 
   return dictionary;
 }
 
 bool IsValidIssueActionTypeNum(int issue_action_type_num) {
   return issue_action_type_num >= 0 &&
-         issue_action_type_num < IssueAction::TYPE_MAX;
+         issue_action_type_num <
+             static_cast<int>(IssueInfo::Action::NUM_VALUES);
 }
 
 // Composes a "learn more" URL. The URL depends on template arguments in |args|.
 // Returns an empty string if |args| is invalid.
 std::string GetLearnMoreUrl(const base::DictionaryValue* args) {
   // TODO(imcheng): The template arguments for determining the learn more URL
   // should come from the Issue object in the browser, not from WebUI.
   int help_page_id = -1;
   if (!args->GetInteger("helpPageId", &help_page_id) || help_page_id < 0) {
     DVLOG(1) << "Invalid help page id.";
@@ skipping 68 matching lines @@
   }
 }
 
 void MediaRouterWebUIMessageHandler::ReturnSearchResult(
     const std::string& sink_id) {
   DVLOG(2) << "ReturnSearchResult";
   web_ui()->CallJavascriptFunctionUnsafe(kReceiveSearchResult,
                                          base::StringValue(sink_id));
 }
 
-void MediaRouterWebUIMessageHandler::UpdateIssue(const Issue* issue) {
+void MediaRouterWebUIMessageHandler::UpdateIssue(const Issue& issue) {
   DVLOG(2) << "UpdateIssue";
-  web_ui()->CallJavascriptFunctionUnsafe(
-      kSetIssue,
-      issue ? *IssueToValue(*issue) : *base::Value::CreateNullValue());
+  web_ui()->CallJavascriptFunctionUnsafe(kSetIssue, *IssueToValue(issue));
+}
+
+void MediaRouterWebUIMessageHandler::ClearIssue() {
+  DVLOG(2) << "ClearIssue";
+  web_ui()->CallJavascriptFunctionUnsafe(kSetIssue,
+                                         *base::Value::CreateNullValue());
 }
 
 void MediaRouterWebUIMessageHandler::UpdateMaxDialogHeight(int height) {
   DVLOG(2) << "UpdateMaxDialogHeight";
   web_ui()->CallJavascriptFunctionUnsafe(kUpdateMaxHeight,
                                          base::FundamentalValue(height));
 }
 
 void MediaRouterWebUIMessageHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
@@ skipping 141 matching lines @@
   if (!IsValidCastModeNum(cast_mode_num)) {
     // TODO(imcheng): Record error condition with UMA.
     DVLOG(1) << "Invalid cast mode: " << cast_mode_num << ". Aborting.";
     return;
   }
 
   MediaRouterUI* media_router_ui =
       static_cast<MediaRouterUI*>(web_ui()->GetController());
   if (media_router_ui->HasPendingRouteRequest()) {
     DVLOG(1) << "UI already has pending route request. Ignoring.";
-    Issue issue(
+    IssueInfo issue(
         l10n_util::GetStringUTF8(IDS_MEDIA_ROUTER_ISSUE_PENDING_ROUTE),
-        std::string(), IssueAction(IssueAction::TYPE_DISMISS),
-        std::vector<IssueAction>(), std::string(), Issue::NOTIFICATION,
-        false, -1);
+        IssueInfo::Action::DISMISS, IssueInfo::Severity::NOTIFICATION);
     media_router_ui_->AddIssue(issue);
     return;
   }
 
   DVLOG(2) << __func__ << ": sink id: " << sink_id
            << ", cast mode: " << cast_mode_num;
 
   // TODO(haibinlu): Pass additional parameters into the CreateRoute request,
   // e.g. low-fps-mirror, user-override. (crbug.com/490364)
   if (!media_router_ui->CreateRoute(
@@ skipping 22 matching lines @@
   pref_service->SetBoolean(prefs::kMediaRouterCloudServicesPrefSet, true);
 }
 
 void MediaRouterWebUIMessageHandler::OnActOnIssue(
     const base::ListValue* args) {
   DVLOG(1) << "OnActOnIssue";
   const base::DictionaryValue* args_dict = nullptr;
   Issue::Id issue_id;
   int action_type_num = -1;
   if (!args->GetDictionary(0, &args_dict) ||
-      !args_dict->GetString("issueId", &issue_id) ||
+      !args_dict->GetInteger("issueId", &issue_id) ||
       !args_dict->GetInteger("actionType", &action_type_num)) {
     DVLOG(1) << "Unable to extract args.";
     return;
   }
   if (!IsValidIssueActionTypeNum(action_type_num)) {
     DVLOG(1) << "Invalid action type: " << action_type_num;
     return;
   }
-  IssueAction::Type action_type =
-      static_cast<IssueAction::Type>(action_type_num);
+  IssueInfo::Action action_type =
+      static_cast<IssueInfo::Action>(action_type_num);
   if (ActOnIssueType(action_type, args_dict))
     DVLOG(1) << "ActOnIssueType failed for Issue ID " << issue_id;
   media_router_ui_->ClearIssue(issue_id);
 }
 
 void MediaRouterWebUIMessageHandler::OnJoinRoute(const base::ListValue* args) {
   DVLOG(1) << "OnJoinRoute";
   const base::DictionaryValue* args_dict = nullptr;
   std::string route_id;
   std::string sink_id;
@@ skipping 13 matching lines @@
   if (route_id.empty()) {
     DVLOG(1) << "Media Route UI did not respond with a "
              << "valid route ID. Aborting.";
     return;
   }
 
   MediaRouterUI* media_router_ui =
       static_cast<MediaRouterUI*>(web_ui()->GetController());
   if (media_router_ui->HasPendingRouteRequest()) {
     DVLOG(1) << "UI already has pending route request. Ignoring.";
-    Issue issue(
+    IssueInfo issue(
         l10n_util::GetStringUTF8(IDS_MEDIA_ROUTER_ISSUE_PENDING_ROUTE),
-        std::string(), IssueAction(IssueAction::TYPE_DISMISS),
-        std::vector<IssueAction>(), std::string(), Issue::NOTIFICATION,
-        false, -1);
+        IssueInfo::Action::DISMISS, IssueInfo::Severity::NOTIFICATION);
     media_router_ui_->AddIssue(issue);
     return;
   }
 
   if (!media_router_ui_->ConnectRoute(sink_id, route_id)) {
     DVLOG(1) << "Error initiating route join request.";
   }
 }
 
 void MediaRouterWebUIMessageHandler::OnCloseRoute(const base::ListValue* args) {
@@ skipping 211 matching lines @@
 }
 
 void MediaRouterWebUIMessageHandler::OnInitialDataReceived(
     const base::ListValue* args) {
   DVLOG(1) << "OnInitialDataReceived";
   media_router_ui_->OnUIInitialDataReceived();
   MaybeUpdateFirstRunFlowData();
 }
 
 bool MediaRouterWebUIMessageHandler::ActOnIssueType(
-    const IssueAction::Type& action_type,
+    IssueInfo::Action action_type,
     const base::DictionaryValue* args) {
-  if (action_type == IssueAction::TYPE_LEARN_MORE) {
+  if (action_type == IssueInfo::Action::LEARN_MORE) {
     std::string learn_more_url = GetLearnMoreUrl(args);
     if (learn_more_url.empty())
       return false;
     std::unique_ptr<base::ListValue> open_args(new base::ListValue);
     open_args->AppendString(learn_more_url);
     web_ui()->CallJavascriptFunctionUnsafe(kWindowOpen, *open_args);
     return true;
   } else {
     // Do nothing; no other issue action types require any other action.
     return true;
@@ skipping 88 matching lines @@
   }
 
   return value;
 }
 
 void MediaRouterWebUIMessageHandler::SetWebUIForTest(content::WebUI* web_ui) {
   set_web_ui(web_ui);
 }
 
 }  // namespace media_router