Roughs in support for feedback deep link.

Deep link launches Chrome OS feedback.

Known issues:
- Feedback UI launches behind Assistant UI.
- Feedback does not attach Assistant server logs.

See bug for demo.

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

ash/assistant/assistant_controller.cc

@@ -28,11 +28,13 @@ AssistantController::AssistantController()
           std::make_unique<AssistantScreenContextController>(this)),
       assistant_ui_controller_(std::make_unique<AssistantUiController>(this)),
       weak_factory_(this) {
+  AddObserver(this);
   NotifyConstructed();
 }
 
 AssistantController::~AssistantController() {
   NotifyDestroying();
+  RemoveObserver(this);
 }
 
 void AssistantController::BindRequest(
@@ -143,6 +145,15 @@ void AssistantController::DownloadImage(
   assistant_image_downloader_->Download(account_id, url, std::move(callback));
 }
 
+void AssistantController::OnDeepLinkReceived(const GURL& deep_link) {
+  // TODO(dmblack): Possibly use a new FeedbackSource (this method defaults to
+  // kFeedbackSourceAsh). This may be useful for differentiating feedback UI and
+  // behavior for Assistant.
+  using assistant::util::DeepLinkType;
+  if (assistant::util::IsDeepLinkType(deep_link, DeepLinkType::kFeedback))
+    Shell::Get()->new_window_controller()->OpenFeedbackPage();
+}
+
 void AssistantController::OnOpenUrlFromTab(const GURL& url) {
   OpenUrl(url);
 }

ash/assistant/assistant_controller.h

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "ash/ash_export.h"
+#include "ash/assistant/assistant_controller_observer.h"
 #include "ash/public/interfaces/assistant_controller.mojom.h"
 #include "ash/public/interfaces/assistant_image_downloader.mojom.h"
 #include "ash/public/interfaces/assistant_setup.mojom.h"
@@ -27,7 +28,6 @@ class UnguessableToken;
 
 namespace ash {
 
-class AssistantControllerObserver;
 class AssistantInteractionController;
 class AssistantNotificationController;
 class AssistantScreenContextController;
@@ -35,6 +35,7 @@ class AssistantUiController;
 
 class ASH_EXPORT AssistantController
     : public mojom::AssistantController,
+      public AssistantControllerObserver,
       public mojom::ManagedWebContentsOpenUrlDelegate {
  public:
   AssistantController();
@@ -84,6 +85,9 @@ class ASH_EXPORT AssistantController
   void RequestScreenshot(const gfx::Rect& rect,
                          RequestScreenshotCallback callback) override;
 
+  // AssistantControllerObserver:
+  void OnDeepLinkReceived(const GURL& deep_link) override;
+
   // mojom::ManagedWebContentsOpenUrlDelegate:
   void OnOpenUrlFromTab(const GURL& url) override;
 

ash/assistant/assistant_interaction_controller.cc

@@ -5,7 +5,6 @@
 #include "ash/assistant/assistant_interaction_controller.h"
 
 #include "ash/assistant/assistant_controller.h"
-#include "ash/assistant/assistant_notification_controller.h"
 #include "ash/assistant/assistant_ui_controller.h"
 #include "ash/assistant/model/assistant_interaction_model_observer.h"
 #include "ash/assistant/model/assistant_query.h"

ash/assistant/util/deep_link_util.cc

@@ -4,6 +4,8 @@
 
 #include "ash/assistant/util/deep_link_util.h"
 
+#include <set>
+
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "url/gurl.h"
@@ -14,7 +16,9 @@ namespace util {
 
 namespace {
 
+// Supported deep link prefixes.
 constexpr char kAssistantExplorePrefix[] = "googleassistant://explore";
+constexpr char kAssistantFeedbackPrefix[] = "googleassistant://send-feedback";
 constexpr char kAssistantOnboardingPrefix[] = "googleassistant://onboarding";
 constexpr char kAssistantRemindersPrefix[] = "googleassistant://reminders";
 constexpr char kAssistantSettingsPrefix[] = "googleassistant://settings";
@@ -43,58 +47,71 @@ constexpr char kAssistantSettingsWebUrl[] = R"(data:text/html,
   </html>
 )";
 
-}  // namespace
+// Map of supported deep link types to their prefixes.
+const std::map<DeepLinkType, std::string> kSupportedDeepLinks = {
+    {DeepLinkType::kExplore, kAssistantExplorePrefix},
+    {DeepLinkType::kFeedback, kAssistantFeedbackPrefix},
+    {DeepLinkType::kOnboarding, kAssistantOnboardingPrefix},
+    {DeepLinkType::kReminders, kAssistantRemindersPrefix},
+    {DeepLinkType::kSettings, kAssistantSettingsPrefix}};
 
-bool IsDeepLinkUrl(const GURL& url) {
-  return IsAssistantExploreDeepLink(url) ||
-         IsAssistantOnboardingDeepLink(url) ||
-         IsAssistantRemindersDeepLink(url) || IsAssistantSettingsDeepLink(url);
-}
+// Set of deep link types which open web contents in the Assistant UI.
+const std::set<DeepLinkType> kWebDeepLinks = {
+    DeepLinkType::kExplore, DeepLinkType::kReminders, DeepLinkType::kSettings};
 
-bool IsAssistantExploreDeepLink(const GURL& url) {
-  return base::StartsWith(url.spec(), kAssistantExplorePrefix,
-                          base::CompareCase::SENSITIVE);
-}
+}  // namespace
 
-bool IsAssistantOnboardingDeepLink(const GURL& url) {
-  return base::StartsWith(url.spec(), kAssistantOnboardingPrefix,
-                          base::CompareCase::SENSITIVE);
+GURL CreateAssistantSettingsDeepLink() {
+  return GURL(kAssistantSettingsPrefix);
 }
 
-bool IsAssistantRemindersDeepLink(const GURL& url) {
-  return base::StartsWith(url.spec(), kAssistantRemindersPrefix,
-                          base::CompareCase::SENSITIVE);
+DeepLinkType GetDeepLinkType(const GURL& url) {
+  for (const auto& supported_deep_link : kSupportedDeepLinks) {
+    if (base::StartsWith(url.spec(), supported_deep_link.second,
+                         base::CompareCase::SENSITIVE)) {
+      return supported_deep_link.first;
+    }
+  }
+  return DeepLinkType::kUnsupported;
 }
 
-GURL CreateAssistantSettingsDeepLink() {
-  return GURL(kAssistantSettingsPrefix);
+bool IsDeepLinkType(const GURL& url, DeepLinkType type) {
+  return GetDeepLinkType(url) == type;
 }
 
-bool IsAssistantSettingsDeepLink(const GURL& url) {
-  return base::StartsWith(url.spec(), kAssistantSettingsPrefix,
-                          base::CompareCase::SENSITIVE);
+bool IsDeepLinkUrl(const GURL& url) {
+  return GetDeepLinkType(url) != DeepLinkType::kUnsupported;
 }
 
 base::Optional<GURL> GetWebUrl(const GURL& deep_link) {
-  if (!IsWebDeepLink(deep_link))
-    return base::nullopt;
-
-  if (IsAssistantExploreDeepLink(deep_link))
-    return GURL(kAssistantExploreWebUrl);
+  DeepLinkType type = GetDeepLinkType(deep_link);
 
-  if (IsAssistantRemindersDeepLink(deep_link))
-    return GURL(kAssistantRemindersWebUrl);
+  if (!IsWebDeepLinkType(type))
+    return base::nullopt;
 
-  if (IsAssistantSettingsDeepLink(deep_link))
-    return GURL(kAssistantSettingsWebUrl);
+  switch (type) {
+    case DeepLinkType::kExplore:
+      return GURL(kAssistantExploreWebUrl);
+    case DeepLinkType::kReminders:
+      return GURL(kAssistantRemindersWebUrl);
+    case DeepLinkType::kSettings:
+      return GURL(kAssistantSettingsWebUrl);
+    case DeepLinkType::kUnsupported:
+    case DeepLinkType::kFeedback:
+    case DeepLinkType::kOnboarding:
+      NOTREACHED();
+      return base::nullopt;
+  }
 
-  NOTIMPLEMENTED();
   return base::nullopt;
 }
 
-bool IsWebDeepLink(const GURL& url) {
-  return IsAssistantExploreDeepLink(url) || IsAssistantRemindersDeepLink(url) ||
-         IsAssistantSettingsDeepLink(url);
+bool IsWebDeepLink(const GURL& deep_link) {
+  return IsWebDeepLinkType(GetDeepLinkType(deep_link));
+}
+
+bool IsWebDeepLinkType(DeepLinkType type) {
+  return kWebDeepLinks.count(type) != 0;
 }
 
 bool ParseDeepLinkParams(const GURL& deep_link,

ash/assistant/util/deep_link_util.h

@@ -17,35 +17,39 @@ namespace ash {
 namespace assistant {
 namespace util {
 
-// Returns true if the specified |url| is a deep link, false otherwise.
-ASH_EXPORT bool IsDeepLinkUrl(const GURL& url);
-
-// Returns true if the specified |url| is an Assistant explore deep link,
-// false otherwise.
-ASH_EXPORT bool IsAssistantExploreDeepLink(const GURL& url);
-
-// Returns true if the specified |url| is an Assistant onboarding deep link,
-// false otherwise.
-ASH_EXPORT bool IsAssistantOnboardingDeepLink(const GURL& url);
-
-// Returns true if the specified |url| is an Assistant reminders deep link,
-// false otherwise.
-ASH_EXPORT bool IsAssistantRemindersDeepLink(const GURL& url);
+// Enumeration of deep link types.
+enum class DeepLinkType {
+  kUnsupported,
+  kExplore,
+  kFeedback,
+  kOnboarding,
+  kReminders,
+  kSettings,
+};
 
 // Returns a deep link to top level Assistant Settings.
 ASH_EXPORT GURL CreateAssistantSettingsDeepLink();
 
-// Returns true if the specified |url| is an Assistant Settings deep link, false
-// otherwise.
-ASH_EXPORT bool IsAssistantSettingsDeepLink(const GURL& url);
+// 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);
+
+// Returns true if the specified |url| is a deep link of the given |type|.
+ASH_EXPORT bool IsDeepLinkType(const GURL& url, DeepLinkType type);
+
+// Returns true if the specified |url| is a deep link, false otherwise.
+ASH_EXPORT bool IsDeepLinkUrl(const GURL& url);
 
 // Returns the web URL for the specified |deep_link|. A return value will only
 // be present if |deep_link| is a web deep link as identified by the
 // IsWebDeepLink(GURL) API.
 ASH_EXPORT base::Optional<GURL> GetWebUrl(const GURL& deep_link);
 
-// Returns true if the specified |url| is a web deep link, false otherwise.
-ASH_EXPORT bool IsWebDeepLink(const GURL& url);
+// 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