Commit 0caf3368 authored by Tommy Martino's avatar Tommy Martino Committed by Chromium LUCI CQ

[SH iOS] Add flag for blocklist and hook into edit menu

This CL hooks the new Disabled Sites code in components into the iOS
Shared Highlighting flow. It also adds a new flag which is used in the
process.

Other platforms can reuse the same flag, but it is not yet exposed via
chrome://flags on those platforms and would need to be added.

Bug: 1157981
Change-Id: I9d84fc4c04fe06fd8e806a98b9ff57bd3f743667
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2626207
Commit-Queue: Tommy Martino <tmartino@chromium.org>
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Auto-Submit: Tommy Martino <tmartino@chromium.org>
Reviewed-by: default avatarRohit Rao <rohitrao@chromium.org>
Reviewed-by: default avatarSebastien Lalancette <seblalancette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843165}
parent eca7becf
......@@ -4438,6 +4438,11 @@
"owners": ["tmartino"],
"expiry_milestone": 90
},
{
"name": "shared-highlighting-use-blocklist",
"owners": ["tmartino"],
"expiry_milestone": 91
},
{
"name": "sharing-device-expiration",
"owners": [ "//chrome/browser/sharing/OWNERS" ],
......
......@@ -6,6 +6,8 @@ static_library("common") {
sources = [
"disabled_sites.cc",
"disabled_sites.h",
"shared_highlighting_features.cc",
"shared_highlighting_features.h",
"shared_highlighting_metrics.cc",
"shared_highlighting_metrics.h",
"text_fragment.cc",
......
......@@ -4,6 +4,8 @@
#include "components/shared_highlighting/core/common/disabled_sites.h"
#include "base/feature_list.h"
#include "components/shared_highlighting/core/common/shared_highlighting_features.h"
#include "third_party/re2/src/re2/re2.h"
#include <map>
......@@ -12,6 +14,7 @@
namespace shared_highlighting {
bool ShouldOfferLinkToText(const GURL& url) {
if (base::FeatureList::IsEnabled(kSharedHighlightingUseBlocklist)) {
// If a URL's host matches a key in this map, then the path will be tested
// against the RE stored in the value. For example, {"foo.com", ".*"} means
// any page on the foo.com domain.
......@@ -20,12 +23,14 @@ bool ShouldOfferLinkToText(const GURL& url) {
// TODO(crbug.com/1157981): special case this to cover other Google TLDs
{"google.com", "^\\/amp\\/.*"}};
const std::string domain =
url.host().compare(0, 4, "www.") == 0 ? url.host().substr(4) : url.host();
const std::string domain = url.host().compare(0, 4, "www.") == 0
? url.host().substr(4)
: url.host();
auto it = kBlocklist.find(domain);
if (it != kBlocklist.end()) {
return !re2::RE2::FullMatch(url.path(), it->second);
}
}
return true;
}
......
......@@ -4,6 +4,8 @@
#include "components/shared_highlighting/core/common/disabled_sites.h"
#include "base/test/scoped_feature_list.h"
#include "components/shared_highlighting/core/common/shared_highlighting_features.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
......@@ -39,5 +41,14 @@ TEST(DisabledSitesTest, NonMatchingHost) {
EXPECT_TRUE(ShouldOfferLinkToText(GURL("https://www.example.com")));
}
TEST(DisabledSitesTest, FeatureDisabled) {
base::test::ScopedFeatureList feature;
feature.InitAndDisableFeature(kSharedHighlightingUseBlocklist);
EXPECT_TRUE(ShouldOfferLinkToText(GURL("https://www.youtube.com")));
EXPECT_TRUE(ShouldOfferLinkToText(GURL("https://www.google.com/amp/")));
EXPECT_TRUE(ShouldOfferLinkToText(GURL("https://www.example.com")));
}
} // namespace
} // namespace shared_highlighting
// Copyright 2021 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 "components/shared_highlighting/core/common/shared_highlighting_features.h"
#include "base/feature_list.h"
namespace shared_highlighting {
const base::Feature kSharedHighlightingUseBlocklist{
"SharedHighlightingUseBlocklist", base::FEATURE_ENABLED_BY_DEFAULT};
} // namespace shared_highlighting
// Copyright 2021 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.
#ifndef COMPONENTS_SHARED_HIGHLIGHTING_CORE_COMMON_SHARED_HIGHLIGHTING_FEATURES_H_
#define COMPONENTS_SHARED_HIGHLIGHTING_CORE_COMMON_SHARED_HIGHLIGHTING_FEATURES_H_
namespace base {
struct Feature;
}
namespace shared_highlighting {
// If enabled, a blocklist will disable link generation on certain pages where
// the feature is unlikely to work correctly.
extern const base::Feature kSharedHighlightingUseBlocklist;
} // namespace shared_highlighting
#endif // COMPONENTS_SHARED_HIGHLIGHTING_CORE_COMMON_SHARED_HIGHLIGHTING_FEATURES_H_
......@@ -84,6 +84,7 @@ include_rules = [
"+components/services/patch",
"+components/services/unzip",
"+components/sessions",
"+components/shared_highlighting",
"+components/signin/core/browser",
"+components/signin/public",
"+components/signin/ios/browser",
......
......@@ -35,6 +35,7 @@ source_set("flags") {
"//components/search_provider_logos",
"//components/security_state/core",
"//components/send_tab_to_self",
"//components/shared_highlighting/core/common",
"//components/signin/core/browser",
"//components/signin/ios/browser",
"//components/signin/public/base",
......
......@@ -45,6 +45,7 @@
#include "components/policy/policy_constants.h"
#include "components/security_state/core/features.h"
#include "components/send_tab_to_self/features.h"
#include "components/shared_highlighting/core/common/shared_highlighting_features.h"
#include "components/signin/core/browser/account_reconcilor.h"
#include "components/signin/ios/browser/features.h"
#include "components/signin/public/base/account_consistency_method.h"
......@@ -598,6 +599,11 @@ const flags_ui::FeatureEntry kFeatureEntries[] = {
flag_descriptions::kOmniboxNewImplementationName,
flag_descriptions::kOmniboxNewImplementationDescription, flags_ui::kOsIos,
FEATURE_VALUE_TYPE(kIOSNewOmniboxImplementation)},
{"shared-highlighting-use-blocklist",
flag_descriptions::kSharedHighlightingUseBlocklistIOSName,
flag_descriptions::kSharedHighlightingUseBlocklistIOSDescription,
flags_ui::kOsIos,
FEATURE_VALUE_TYPE(shared_highlighting::kSharedHighlightingUseBlocklist)},
};
bool SkipConditionalFeatureEntry(const flags_ui::FeatureEntry& entry) {
......
......@@ -259,6 +259,14 @@ const char kIOSSharedHighlightingColorChangeDescription[] =
"Changes the Shared Highlighting color of the text fragment"
"away from the default yellow in iOS. Works with #scroll-to-text-ios flag.";
const char kSharedHighlightingUseBlocklistIOSName[] =
"Shared Highlighting blocklist";
const char kSharedHighlightingUseBlocklistIOSDescription[] =
"Uses a blocklist to disable Shared Highlighting link generation on "
"certain sites where personalized or dynamic content or other technical "
"restrictions make it unlikely that a URL can be generated and actually "
"work when shared.";
const char kLocationPermissionsPromptName[] =
"Location Permisssions Prompt Experiment";
const char kLocationPermissionsPromptDescription[] =
......
......@@ -334,6 +334,11 @@ extern const char kSettingsRefreshDescription[];
extern const char kSharedHighlightingIOSName[];
extern const char kSharedHighlightingIOSDescription[];
// Title and description for the flag to use a sites blocklist when generating
// URLs for Shared Highlighting (Link to Text).
extern const char kSharedHighlightingUseBlocklistIOSName[];
extern const char kSharedHighlightingUseBlocklistIOSDescription[];
// Title and description for the flag to enable annotating web forms with
// Autofill field type predictions as placeholder.
extern const char kShowAutofillTypePredictionsName[];
......
......@@ -8,6 +8,7 @@
#import "base/optional.h"
#import "base/timer/elapsed_timer.h"
#import "base/values.h"
#import "components/shared_highlighting/core/common/disabled_sites.h"
#import "ios/chrome/browser/link_to_text/link_to_text_constants.h"
#import "ios/web/public/js_messaging/web_frame.h"
#import "ios/web/public/js_messaging/web_frames_manager.h"
......@@ -40,7 +41,11 @@ void LinkToTextTabHelper::CreateForWebState(web::WebState* web_state) {
}
bool LinkToTextTabHelper::ShouldOffer() {
// TODO(crbug.com/1134708): add more checks, like text only.
if (!shared_highlighting::ShouldOfferLinkToText(
web_state_->GetLastCommittedURL())) {
return false;
}
web::WebFrame* main_frame =
web_state_->GetWebFramesManager()->GetMainWebFrame();
return web_state_->ContentIsHTML() && main_frame &&
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment