Pass type and params in OnDeepLinkReceived event.

DeepLinkType enum is more easily consumed by event observers.
DeepLinkType + params adequately represents the complete deep link.

Bug: b:111437390
Change-Id: I62618b9ae84a1240232bd2f9e50f999cb2be9055
Reviewed-on: https://chromium-review.googlesource.com/1136855
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575075}

ash/assistant/assistant_controller.cc

@@ -145,20 +145,28 @@ void AssistantController::DownloadImage(
   assistant_image_downloader_->Download(account_id, url, std::move(callback));
 }
 
-void AssistantController::OnDeepLinkReceived(const GURL& deep_link) {
-  using assistant::util::DeepLinkType;
-
-  // TODO(dmblack): Possibly use a new FeedbackSource (this method defaults to
-  // kFeedbackSourceAsh). This may be useful for differentiating feedback UI and
-  // behavior for Assistant.
-  if (assistant::util::IsDeepLinkType(deep_link, DeepLinkType::kFeedback))
-    return Shell::Get()->new_window_controller()->OpenFeedbackPage();
-
-  // TODO(updowndota): Pass any parameters necessary to |assistant_setup_| that
-  // it requires to relaunch Assistant UI on completion of opt in flow.
-  if (assistant::util::IsDeepLinkType(deep_link, DeepLinkType::kOnboarding)) {
-    assistant_setup_->StartAssistantOptInFlow();
-    assistant_ui_controller_->HideUi(AssistantSource::kDeepLink);
+void AssistantController::OnDeepLinkReceived(
+    assistant::util::DeepLinkType type,
+    const std::map<std::string, std::string>& params) {
+  switch (type) {
+    case assistant::util::DeepLinkType::kFeedback:
+      // TODO(dmblack): Possibly use a new FeedbackSource (this method defaults
+      // to kFeedbackSourceAsh). This may be useful for differentiating feedback
+      // UI and behavior for Assistant.
+      Shell::Get()->new_window_controller()->OpenFeedbackPage();
+      break;
+    case assistant::util::DeepLinkType::kOnboarding:
+      // TODO(updowndota): Pass any |params| necessary to |assistant_setup_|
+      // that it requires to relaunch Assistant UI on completion of opt in flow.
+      assistant_setup_->StartAssistantOptInFlow();
+      assistant_ui_controller_->HideUi(AssistantSource::kDeepLink);
+      break;
+    case assistant::util::DeepLinkType::kUnsupported:
+    case assistant::util::DeepLinkType::kExplore:
+    case assistant::util::DeepLinkType::kReminders:
+    case assistant::util::DeepLinkType::kSettings:
+      // No action needed.
+      break;
   }
 }
 
@@ -187,8 +195,15 @@ void AssistantController::NotifyDestroying() {
 }
 
 void AssistantController::NotifyDeepLinkReceived(const GURL& deep_link) {
+  using namespace assistant::util;
+
+  // Retrieve deep link type and parsed parameters.
+  DeepLinkType type = GetDeepLinkType(deep_link);
+  const std::map<std::string, std::string> params =
+      GetDeepLinkParams(deep_link);
+
   for (AssistantControllerObserver& observer : observers_)
-    observer.OnDeepLinkReceived(deep_link);
+    observer.OnDeepLinkReceived(type, params);
 }
 
 void AssistantController::NotifyUrlOpened(const GURL& url) {

ash/assistant/assistant_controller.h

@@ -86,7 +86,9 @@ class ASH_EXPORT AssistantController
                          RequestScreenshotCallback callback) override;
 
   // AssistantControllerObserver:
-  void OnDeepLinkReceived(const GURL& deep_link) override;
+  void OnDeepLinkReceived(
+      assistant::util::DeepLinkType type,
+      const std::map<std::string, std::string>& params) override;
 
   // mojom::ManagedWebContentsOpenUrlDelegate:
   void OnOpenUrlFromTab(const GURL& url) override;

ash/assistant/assistant_controller_observer.h

@@ -5,12 +5,21 @@
 #ifndef ASH_ASSISTANT_ASSISTANT_CONTROLLER_OBSERVER_H_
 #define ASH_ASSISTANT_ASSISTANT_CONTROLLER_OBSERVER_H_
 
+#include <map>
+#include <string>
+
 #include "base/macros.h"
 
 class GURL;
 
 namespace ash {
 
+namespace assistant {
+namespace util {
+enum class DeepLinkType;
+}  // namespace util
+}  // namespace assistant
+
 class AssistantControllerObserver {
  public:
   // Invoked when the AssistantController has been fully constructed.
@@ -19,8 +28,11 @@ class AssistantControllerObserver {
   // Invoked when the AssistantController is starting to be destroyed.
   virtual void OnAssistantControllerDestroying() {}
 
-  // Invoked when Assistant has received the specified |deep_link|.
-  virtual void OnDeepLinkReceived(const GURL& deep_link) {}
+  // Invoked when Assistant has received a deep link of the specified |type|
+  // with the given |params|.
+  virtual void OnDeepLinkReceived(
+      assistant::util::DeepLinkType type,
+      const std::map<std::string, std::string>& params) {}
 
   // Invoked when the specified |url| is opened by Assistant in a new tab.
   virtual void OnUrlOpened(const GURL& url) {}

ash/assistant/assistant_ui_controller.cc

@@ -181,8 +181,10 @@ void AssistantUiController::OnHighlighterEnabledChanged(
   }
 }
 
-void AssistantUiController::OnDeepLinkReceived(const GURL& url) {
-  if (!assistant::util::IsWebDeepLink(url))
+void AssistantUiController::OnDeepLinkReceived(
+    assistant::util::DeepLinkType type,
+    const std::map<std::string, std::string>& params) {
+  if (!assistant::util::IsWebDeepLinkType(type))
     return;
 
   ShowUi(AssistantSource::kDeepLink);

ash/assistant/assistant_ui_controller.h

@@ -90,7 +90,9 @@ class ASH_EXPORT AssistantUiController
   // AssistantControllerObserver:
   void OnAssistantControllerConstructed() override;
   void OnAssistantControllerDestroying() override;
-  void OnDeepLinkReceived(const GURL& deep_link) override;
+  void OnDeepLinkReceived(
+      assistant::util::DeepLinkType type,
+      const std::map<std::string, std::string>& params) override;
   void OnUrlOpened(const GURL& url) override;
 
   // AssistantUiModelObserver:

ash/assistant/ui/assistant_web_view.cc

@@ -59,25 +59,28 @@ void AssistantWebView::InitLayout() {
   AddChildView(caption_bar_);
 }
 
-void AssistantWebView::OnDeepLinkReceived(const GURL& deep_link) {
-  if (!assistant::util::IsWebDeepLink(deep_link))
+void AssistantWebView::OnDeepLinkReceived(
+    assistant::util::DeepLinkType type,
+    const std::map<std::string, std::string>& params) {
+  if (!assistant::util::IsWebDeepLinkType(type))
     return;
 
   ReleaseWebContents();
 
   web_contents_id_token_ = base::UnguessableToken::Create();
 
-  const int preferred_height_dip =
-      kMaxHeightDip - caption_bar_->GetPreferredSize().height();
+  gfx::Size preferred_size_dip =
+      gfx::Size(kPreferredWidthDip,
+                kMaxHeightDip - caption_bar_->GetPreferredSize().height());
 
-  ash::mojom::ManagedWebContentsParamsPtr params(
+  ash::mojom::ManagedWebContentsParamsPtr web_contents_params(
       ash::mojom::ManagedWebContentsParams::New());
-  params->url = assistant::util::GetWebUrl(deep_link).value();
-  params->min_size_dip = gfx::Size(kPreferredWidthDip, preferred_height_dip);
-  params->max_size_dip = gfx::Size(kPreferredWidthDip, preferred_height_dip);
+  web_contents_params->url = GetWebUrl(type).value();
+  web_contents_params->min_size_dip = preferred_size_dip;
+  web_contents_params->max_size_dip = preferred_size_dip;
 
   assistant_controller_->ManageWebContents(
-      web_contents_id_token_.value(), std::move(params),
+      web_contents_id_token_.value(), std::move(web_contents_params),
       base::BindOnce(&AssistantWebView::OnWebContentsReady,
                      web_contents_request_factory_.GetWeakPtr()));
 }

ash/assistant/ui/assistant_web_view.h

@@ -37,7 +37,9 @@ class AssistantWebView : public views::View,
   void ChildPreferredSizeChanged(views::View* child) override;
 
   // AssistantControllerObserver:
-  void OnDeepLinkReceived(const GURL& deep_link) override;
+  void OnDeepLinkReceived(
+      assistant::util::DeepLinkType type,
+      const std::map<std::string, std::string>& params) override;
 
  private:
   void InitLayout();

ash/assistant/util/deep_link_util.cc

@@ -65,6 +65,29 @@ GURL CreateAssistantSettingsDeepLink() {
   return GURL(kAssistantSettingsPrefix);
 }
 
+std::map<std::string, std::string> GetDeepLinkParams(const GURL& deep_link) {
+  std::map<std::string, std::string> params;
+
+  if (!IsDeepLinkUrl(deep_link))
+    return params;
+
+  if (!deep_link.has_query())
+    return params;
+
+  // Key-value pairs are '&' delimited and the keys/values are '=' delimited.
+  // Example: "googleassistant://onboarding?k1=v1&k2=v2".
+  base::StringPairs pairs;
+  if (!base::SplitStringIntoKeyValuePairs(deep_link.query(), '=', '&',
+                                          &pairs)) {
+    return params;
+  }
+
+  for (const auto& pair : pairs)
+    params[pair.first] = pair.second;
+
+  return params;
+};
+
 DeepLinkType GetDeepLinkType(const GURL& url) {
   for (const auto& supported_deep_link : kSupportedDeepLinks) {
     if (base::StartsWith(url.spec(), supported_deep_link.second,
@@ -84,8 +107,10 @@ bool IsDeepLinkUrl(const GURL& url) {
 }
 
 base::Optional<GURL> GetWebUrl(const GURL& deep_link) {
-  DeepLinkType type = GetDeepLinkType(deep_link);
+  return GetWebUrl(GetDeepLinkType(deep_link));
+}
 
+base::Optional<GURL> GetWebUrl(DeepLinkType type) {
   if (!IsWebDeepLinkType(type))
     return base::nullopt;
 
@@ -114,35 +139,6 @@ bool IsWebDeepLinkType(DeepLinkType type) {
   return kWebDeepLinks.count(type) != 0;
 }
 
-bool ParseDeepLinkParams(const GURL& deep_link,
-                         std::map<std::string, std::string>& params) {
-  if (!IsDeepLinkUrl(deep_link))
-    return false;
-
-  // If the deep link does not have a query then there are no key-value pairs to
-  // be parsed. We still return true in this case because omission of parameters
-  // in a deep link does not cause a parse attempt to fail.
-  if (!deep_link.has_query())
-    return true;
-
-  // Key-value pairs are '&' delimited and the keys/values are '=' delimited.
-  // Example: "googleassistant://onboarding?k1=v1&k2=v2".
-  base::StringPairs pairs;
-  if (!base::SplitStringIntoKeyValuePairs(deep_link.query(), '=', '&',
-                                          &pairs)) {
-    return false;
-  }
-
-  // Insert the parsed values into the caller provided map of |params|.
-  for (std::pair<std::string, std::string>& pair : pairs) {
-    if (params.count(pair.first) > 0) {
-      params[pair.first] = pair.second;
-    }
-  }
-
-  return true;
-}
-
 }  // namespace util
 }  // namespace assistant
 }  // namespace ash

ash/assistant/util/deep_link_util.h

@@ -30,6 +30,12 @@ enum class DeepLinkType {
 // Returns a deep link to top level Assistant Settings.
 ASH_EXPORT GURL CreateAssistantSettingsDeepLink();
 
+// Returns the parsed parameters for the specified |deep_link|. If the supplied
+// argument is not a supported deep link or if no parameters are found, an empty
+// map is returned.
+ASH_EXPORT std::map<std::string, std::string> GetDeepLinkParams(
+    const GURL& deep_link);
+
 // Returns the deep link type of the specified |url|. If the specified url is
 // not a supported deep link, DeepLinkType::kUnsupported is returned.
 ASH_EXPORT DeepLinkType GetDeepLinkType(const GURL& url);
@@ -45,21 +51,17 @@ ASH_EXPORT bool IsDeepLinkUrl(const GURL& url);
 // IsWebDeepLink(GURL) API.
 ASH_EXPORT base::Optional<GURL> GetWebUrl(const GURL& deep_link);
 
+// Returns the web URL for a deep link of the specified |type|. A return value
+// will only be present if the deep link type is a web deep link type as
+// identified by the IsWebDeepLinkType(DeepLinkType) API.
+ASH_EXPORT base::Optional<GURL> GetWebUrl(DeepLinkType type);
+
 // Returns true if the specified |deep_link| is a web deep link.
 ASH_EXPORT bool IsWebDeepLink(const GURL& deep_link);
 
 // Returns true if the specified deep link |type| is a web deep link.
 ASH_EXPORT bool IsWebDeepLinkType(DeepLinkType type);
 
-// Parses the specified |deep_link| for the values corresponding to the keys in
-// |params|. If a value for a key in |params| is found, it is inserted into the
-// map. If not, the existing value in the map is untouched, so it is recommended
-// to initiate the map with default values prior to utilizing this API. The
-// return value is true if the deep link was successfully parse, false
-// otherwise.
-ASH_EXPORT bool ParseDeepLinkParams(const GURL& deep_link,
-                                    std::map<std::string, std::string>& params);
-
 }  // namespace util
 }  // namespace assistant
 }  // namespace ash

ash/assistant/util/deep_link_util_unittest.cc

@@ -31,6 +31,36 @@ TEST_F(DeepLinkUnitTest, CreateAssistantSettingsDeepLink) {
             CreateAssistantSettingsDeepLink());
 }
 
+TEST_F(DeepLinkUnitTest, GetDeepLinkParams) {
+  std::map<std::string, std::string> params;
+
+  auto ParseDeepLinkParams = [&params](const std::string& url) {
+    params = GetDeepLinkParams(GURL(url));
+  };
+
+  // OK: Supported deep link w/ parameters.
+  ParseDeepLinkParams("googleassistant://onboarding?k1=v1&k2=v2");
+  ASSERT_EQ(2, static_cast<int>(params.size()));
+  ASSERT_EQ("v1", params["k1"]);
+  ASSERT_EQ("v2", params["k2"]);
+
+  // OK: Supported deep link w/o parameters.
+  ParseDeepLinkParams("googleassistant://onboarding");
+  ASSERT_TRUE(params.empty());
+
+  // FAIL: Unsupported deep link.
+  ParseDeepLinkParams("googleassistant://unsupported?k1=v1&k2=v2");
+  ASSERT_TRUE(params.empty());
+
+  // FAIL: Non-deep link URLs.
+  ParseDeepLinkParams("https://www.google.com/search?q=query");
+  ASSERT_TRUE(params.empty());
+
+  // FAIL: Empty URLs.
+  ParseDeepLinkParams(std::string());
+  ASSERT_TRUE(params.empty());
+}
+
 TEST_F(DeepLinkUnitTest, GetDeepLinkType) {
   const std::map<std::string, DeepLinkType> test_cases = {
       // OK: Supported deep links.
@@ -167,6 +197,25 @@ TEST_F(DeepLinkUnitTest, GetWebUrl) {
     ASSERT_EQ(test_case.second, GetWebUrl(GURL(test_case.first)).has_value());
 }
 
+// TODO(dmblack): Assert actual web URLs when available.
+TEST_F(DeepLinkUnitTest, GetWebUrlByType) {
+  const std::map<DeepLinkType, bool> test_cases = {
+      // OK: Supported web deep link types.
+      {DeepLinkType::kExplore, true},
+      {DeepLinkType::kReminders, true},
+      {DeepLinkType::kSettings, true},
+
+      // FAIL: Non-web deep link types.
+      {DeepLinkType::kFeedback, false},
+      {DeepLinkType::kOnboarding, false},
+
+      // FAIL: Unsupported deep link types.
+      {DeepLinkType::kUnsupported, false}};
+
+  for (const auto& test_case : test_cases)
+    ASSERT_EQ(test_case.second, GetWebUrl(test_case.first).has_value());
+}
+
 TEST_F(DeepLinkUnitTest, IsWebDeepLink) {
   const std::map<std::string, bool> test_cases = {
       // OK: Supported web deep links.
@@ -214,88 +263,6 @@ TEST_F(DeepLinkUnitTest, IsWebDeepLinkType) {
     ASSERT_EQ(test_case.second, IsWebDeepLinkType(test_case.first));
 }
 
-TEST_F(DeepLinkUnitTest, ParseDeepLinkParams) {
-  auto AssertParamsParsed = [](const std::string& deep_link,
-                               std::map<std::string, std::string>& params) {
-    ASSERT_TRUE(ParseDeepLinkParams(GURL(deep_link), params));
-  };
-
-  auto AssertParamsNotParsed = [](const std::string& deep_link,
-                                  std::map<std::string, std::string>& params) {
-    ASSERT_FALSE(ParseDeepLinkParams(GURL(deep_link), params));
-  };
-
-  std::map<std::string, std::string> params;
-
-  auto ResetParams = [&params]() {
-    params["k1"] = "default";
-    params["k2"] = "default";
-  };
-
-  ResetParams();
-
-  // OK: All parameters present.
-  AssertParamsParsed("googleassistant://onboarding?k1=v1&k2=v2", params);
-  ASSERT_EQ("v1", params["k1"]);
-  ASSERT_EQ("v2", params["k2"]);
-
-  ResetParams();
-
-  // OK: Some parameters present.
-  AssertParamsParsed("googleassistant://onboarding?k1=v1", params);
-  ASSERT_EQ("v1", params["k1"]);
-  ASSERT_EQ("default", params["k2"]);
-
-  ResetParams();
-
-  // OK: Unknown parameters present.
-  AssertParamsParsed("googleassistant://onboarding?k1=v1&k3=v3", params);
-  ASSERT_EQ("v1", params["k1"]);
-  ASSERT_EQ("default", params["k2"]);
-
-  ResetParams();
-
-  // OK: No parameters present.
-  AssertParamsParsed("googleassistant://onboarding", params);
-  ASSERT_EQ("default", params["k1"]);
-  ASSERT_EQ("default", params["k2"]);
-
-  ResetParams();
-
-  // OK: Parameters are case sensitive.
-  AssertParamsParsed("googleassistant://onboarding?K1=v1", params);
-  ASSERT_EQ("default", params["k1"]);
-  ASSERT_EQ("default", params["k2"]);
-
-  ResetParams();
-
-  // FAIL: Deep links are case sensitive.
-  AssertParamsNotParsed("GOOGLEASSISTANT://ONBOARDING?k1=v1", params);
-  ASSERT_EQ("default", params["k1"]);
-  ASSERT_EQ("default", params["k2"]);
-
-  ResetParams();
-
-  // FAIL: Malformed parameters.
-  AssertParamsNotParsed("googleassistant://onboarding?k1=", params);
-  ASSERT_EQ("default", params["k1"]);
-  ASSERT_EQ("default", params["k2"]);
-
-  ResetParams();
-
-  // FAIL: Non-deep link URLs.
-  AssertParamsNotParsed("https://www.google.com/search?q=query", params);
-  ASSERT_EQ("default", params["k1"]);
-  ASSERT_EQ("default", params["k2"]);
-
-  ResetParams();
-
-  // FAIL: Empty URLs.
-  AssertParamsNotParsed(std::string(), params);
-  ASSERT_EQ("default", params["k1"]);
-  ASSERT_EQ("default", params["k2"]);
-}
-
 }  // namespace util
 }  // namespace assistant
 }  // namespace ash