[OsSettingsDeepLinking] Add settingId to url

Added helper function GetModifiedUrlWithSettingId that returns
the modified url with settingId appended. Also added a testing
file for that function.

Change-Id: Ic140bc81c334b89b43a332b66251cfff199f5a6a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2283653
Commit-Queue: Daniel Classon <dclasson@google.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786519}

chrome/browser/chromeos/BUILD.gn

@@ -3473,6 +3473,7 @@ source_set("unit_tests") {
     "../ui/webui/settings/chromeos/internet_handler_unittest.cc",
     "../ui/webui/settings/chromeos/multidevice_handler_unittest.cc",
     "../ui/webui/settings/chromeos/os_settings_manager_unittest.cc",
+    "../ui/webui/settings/chromeos/os_settings_section_unittest.cc",
     "../ui/webui/settings/chromeos/search/search_handler_unittest.cc",
     "../ui/webui/settings/chromeos/search/search_tag_registry_unittest.cc",
     "../ui/webui/settings/chromeos/search/settings_user_action_tracker_unittest.cc",

chrome/browser/ui/webui/settings/chromeos/internet_section.cc

@@ -786,23 +786,26 @@ std::string InternetSection::ModifySearchResultUrl(
     mojom::SearchResultType type,
     OsSettingsIdentifier id,
     const std::string& url_to_modify) const {
+  std::string modified_url =
+      OsSettingsSection::ModifySearchResultUrl(type, id, url_to_modify);
+
   if (IsPartOfDetailsSubpage(type, id, mojom::Subpage::kEthernetDetails))
-    return GetDetailsSubpageUrl(url_to_modify, *connected_ethernet_guid_);
+    return GetDetailsSubpageUrl(modified_url, *connected_ethernet_guid_);
 
   if (IsPartOfDetailsSubpage(type, id, mojom::Subpage::kWifiDetails))
-    return GetDetailsSubpageUrl(url_to_modify, *connected_wifi_guid_);
+    return GetDetailsSubpageUrl(modified_url, *connected_wifi_guid_);
 
   if (IsPartOfDetailsSubpage(type, id, mojom::Subpage::kCellularDetails))
-    return GetDetailsSubpageUrl(url_to_modify, *connected_cellular_guid_);
+    return GetDetailsSubpageUrl(modified_url, *connected_cellular_guid_);
 
   if (IsPartOfDetailsSubpage(type, id, mojom::Subpage::kTetherDetails))
-    return GetDetailsSubpageUrl(url_to_modify, *connected_tether_guid_);
+    return GetDetailsSubpageUrl(modified_url, *connected_tether_guid_);
 
   if (IsPartOfDetailsSubpage(type, id, mojom::Subpage::kVpnDetails))
-    return GetDetailsSubpageUrl(url_to_modify, *connected_vpn_guid_);
+    return GetDetailsSubpageUrl(modified_url, *connected_vpn_guid_);
 
-  // URL does not need to be modified; use default implementation.
-  return OsSettingsSection::ModifySearchResultUrl(type, id, url_to_modify);
+  // Use default implementation.
+  return modified_url;
 }
 
 void InternetSection::OnDeviceStateListChanged() {

chrome/browser/ui/webui/settings/chromeos/main_section.cc

@@ -123,6 +123,9 @@ void MainSection::AddLoadTimeData(content::WebUIDataSource* html_source) {
       user_manager::UserManager::Get()->IsLoggedInAsAnyKioskApp());
   html_source->AddBoolean("isSupervised", profile()->IsSupervised());
 
+  html_source->AddBoolean("isDeepLinkingEnabled",
+                          chromeos::features::IsDeepLinkingEnabled());
+
   // Add the System Web App resources for Settings.
   if (web_app::SystemWebAppManager::IsEnabled()) {
     html_source->AddResourcePath("icon-192.png", IDR_SETTINGS_LOGO_192);

chrome/browser/ui/webui/settings/chromeos/os_settings_section.cc

@@ -8,11 +8,15 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/system/sys_info.h"
 #include "chrome/grit/generated_resources.h"
+#include "chromeos/constants/chromeos_features.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace chromeos {
 namespace settings {
 
+// static
+constexpr const char OsSettingsSection::kSettingIdUrlParam[];
+
 // static
 base::string16 OsSettingsSection::GetHelpUrlWithBoard(
     const std::string& original_url) {
@@ -44,8 +48,7 @@ std::string OsSettingsSection::ModifySearchResultUrl(
     mojom::SearchResultType type,
     OsSettingsIdentifier id,
     const std::string& url_to_modify) const {
-  // Default case for static URLs which do not need to be modified.
-  return url_to_modify;
+  return GetDefaultModifiedUrl(type, id, url_to_modify);
 }
 
 mojom::SearchResultPtr OsSettingsSection::GenerateSectionSearchResult(
@@ -66,5 +69,29 @@ mojom::SearchResultPtr OsSettingsSection::GenerateSectionSearchResult(
       mojom::SearchResultIdentifier::NewSection(GetSection()));
 }
 
+// static
+std::string OsSettingsSection::GetDefaultModifiedUrl(
+    mojom::SearchResultType type,
+    OsSettingsIdentifier id,
+    const std::string& url_to_modify) {
+  if (!chromeos::features::IsDeepLinkingEnabled() ||
+      type != mojom::SearchResultType::kSetting) {
+    // Default case for static URLs which do not need to be modified.
+    return url_to_modify;
+  }
+
+  std::stringstream ss;
+  ss << url_to_modify;
+  // Handle existing query parameters.
+  if (url_to_modify.find('?') == std::string::npos) {
+    ss << '?';
+  } else {
+    ss << '&';
+  }
+  // Add deep link to query i.e. "settingId=4".
+  ss << kSettingIdUrlParam << '=' << static_cast<int32_t>(id.setting);
+  return ss.str();
+}
+
 }  // namespace settings
 }  // namespace chromeos

chrome/browser/ui/webui/settings/chromeos/os_settings_section.h

@@ -154,6 +154,22 @@ class OsSettingsSection {
   SearchTagRegistry* registry() { return search_tag_registry_; }
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(OsSettingsSectionTest, SectionWithFlag);
+  FRIEND_TEST_ALL_PREFIXES(OsSettingsSectionTest, SectionNoFlag);
+  FRIEND_TEST_ALL_PREFIXES(OsSettingsSectionTest, SubpageWithFlag);
+  FRIEND_TEST_ALL_PREFIXES(OsSettingsSectionTest, SubpageNoFlag);
+  FRIEND_TEST_ALL_PREFIXES(OsSettingsSectionTest, SettingWithFlag);
+  FRIEND_TEST_ALL_PREFIXES(OsSettingsSectionTest, SettingExistingQueryWithFlag);
+  FRIEND_TEST_ALL_PREFIXES(OsSettingsSectionTest, SettingNoFlag);
+
+  static constexpr char kSettingIdUrlParam[] = "settingId";
+
+  // If type is Setting, adds the kSettingIdUrlParam to the query parameter
+  // and returns the deep linked url. Doesn't modify url otherwise.
+  static std::string GetDefaultModifiedUrl(mojom::SearchResultType type,
+                                           OsSettingsIdentifier id,
+                                           const std::string& url_to_modify);
+
   Profile* profile_;
   SearchTagRegistry* search_tag_registry_;
 };

chrome/browser/ui/webui/settings/chromeos/os_settings_section_unittest.cc

+// Copyright 2020 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/settings/chromeos/os_settings_section.h"
+
+#include "base/test/scoped_feature_list.h"
+#include "chrome/browser/ui/webui/settings/chromeos/constants/setting.mojom.h"
+#include "chromeos/constants/chromeos_features.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+namespace settings {
+
+TEST(OsSettingsSectionTest, SectionWithFlag) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      chromeos::features::kOsSettingsDeepLinking);
+
+  // Sections should not incur modification.
+  EXPECT_EQ("internet", OsSettingsSection::GetDefaultModifiedUrl(
+                            /*type=*/mojom::SearchResultType::kSection,
+                            /*id=*/{.section = mojom::Section::kNetwork},
+                            /*url_to_modify=*/"internet"));
+}
+
+TEST(OsSettingsSectionTest, SectionNoFlag) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndDisableFeature(
+      chromeos::features::kOsSettingsDeepLinking);
+
+  // Deep linking disabled, should not modify.
+  EXPECT_EQ("internet", OsSettingsSection::GetDefaultModifiedUrl(
+                            /*type=*/mojom::SearchResultType::kSection,
+                            /*id=*/{.section = mojom::Section::kNetwork},
+                            /*url_to_modify=*/"internet"));
+}
+
+TEST(OsSettingsSectionTest, SubpageWithFlag) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      chromeos::features::kOsSettingsDeepLinking);
+
+  // Subpages should not incur modification.
+  EXPECT_EQ("networks?type=WiFi",
+            OsSettingsSection::GetDefaultModifiedUrl(
+                /*type=*/mojom::SearchResultType::kSubpage,
+                /*id=*/{.subpage = mojom::Subpage::kWifiNetworks},
+                /*url_to_modify=*/"networks?type=WiFi"));
+}
+
+TEST(OsSettingsSectionTest, SubpageNoFlag) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndDisableFeature(
+      chromeos::features::kOsSettingsDeepLinking);
+
+  // Deep linking disabled, should not modify.
+  EXPECT_EQ("networks?type=WiFi",
+            OsSettingsSection::GetDefaultModifiedUrl(
+                /*type=*/mojom::SearchResultType::kSubpage,
+                /*id=*/{.subpage = mojom::Subpage::kWifiNetworks},
+                /*url_to_modify=*/"networks?type=WiFi"));
+}
+
+TEST(OsSettingsSectionTest, SettingWithFlag) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      chromeos::features::kOsSettingsDeepLinking);
+
+  // Settings should have settingId added
+  EXPECT_EQ("networks?settingId=4",
+            OsSettingsSection::GetDefaultModifiedUrl(
+                /*type=*/mojom::SearchResultType::kSetting,
+                /*id=*/{.setting = mojom::Setting::kWifiOnOff},
+                /*url_to_modify=*/"networks"));
+}
+
+TEST(OsSettingsSectionTest, SettingExistingQueryWithFlag) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      chromeos::features::kOsSettingsDeepLinking);
+
+  // Settings with existing query parameters should have settingId added.
+  EXPECT_EQ("networks?type=WiFi&settingId=4",
+            OsSettingsSection::GetDefaultModifiedUrl(
+                /*type=*/mojom::SearchResultType::kSetting,
+                /*id=*/{.setting = mojom::Setting::kWifiOnOff},
+                /*url_to_modify=*/"networks?type=WiFi"));
+}
+
+TEST(OsSettingsSectionTest, SettingNoFlag) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndDisableFeature(
+      chromeos::features::kOsSettingsDeepLinking);
+
+  // Deep linking disabled, should not modify.
+  EXPECT_EQ("networks?type=WiFi",
+            OsSettingsSection::GetDefaultModifiedUrl(
+                /*type=*/mojom::SearchResultType::kSetting,
+                /*id=*/{.setting = mojom::Setting::kWifiOnOff},
+                /*url_to_modify=*/"networks?type=WiFi"));
+}
+
+}  // namespace settings
+}  // namespace chromeos