Skip to content

Commit

Permalink
[send-tab-to-self/ios] Share logic to check entry point state
Browse files Browse the repository at this point in the history
No behavior is changed. Move it to //components so it can be called
on iOS.

Bug: 1264471
Change-Id: I1fc5cd31339f016057779737910c5ab54028cb67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3637732
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002610}
  • Loading branch information
Victor Hugo Vianna Silva authored and Chromium LUCI CQ committed May 12, 2022
1 parent c931620 commit f2271ab
Show file tree
Hide file tree
Showing 14 changed files with 336 additions and 377 deletions.
48 changes: 7 additions & 41 deletions chrome/browser/send_tab_to_self/send_tab_to_self_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,53 +19,19 @@

namespace send_tab_to_self {

namespace {

bool ShouldOfferSignin(Profile* profile) {
syncer::SyncService* sync_service =
SyncServiceFactory::GetForProfile(profile);
return profile->GetPrefs()->GetBoolean(prefs::kSigninAllowed) &&
sync_service && sync_service->GetAccountInfo().IsEmpty() &&
!sync_service->IsLocalSyncEnabled();
}

} // namespace

absl::optional<EntryPointDisplayReason> GetEntryPointDisplayReason(
content::WebContents* web_contents) {
// TODO(crbug.com/1274173): This can probably be a DCHECK instead.
if (!web_contents)
return absl::nullopt;

if (!web_contents->GetLastCommittedURL().SchemeIsHTTPOrHTTPS())
return absl::nullopt;

Profile* profile =
auto* profile =
Profile::FromBrowserContext(web_contents->GetBrowserContext());
SendTabToSelfSyncService* send_tab_to_self_sync_service =
SendTabToSelfSyncServiceFactory::GetForProfile(profile);
if (!send_tab_to_self_sync_service) {
// Can happen in incognito or guest profile.
return absl::nullopt;
}

if (ShouldOfferSignin(profile) &&
base::FeatureList::IsEnabled(kSendTabToSelfSigninPromo)) {
return EntryPointDisplayReason::kOfferSignIn;
}

SendTabToSelfModel* model =
send_tab_to_self_sync_service->GetSendTabToSelfModel();
if (!model->IsReady())
return absl::nullopt;

if (!model->HasValidTargetDevice()) {
return base::FeatureList::IsEnabled(kSendTabToSelfSigninPromo)
? absl::make_optional(
EntryPointDisplayReason::kInformNoTargetDevice)
: absl::nullopt;
}

return EntryPointDisplayReason::kOfferFeature;
return GetEntryPointDisplayReason(
web_contents->GetLastCommittedURL(),
SyncServiceFactory::GetForProfile(profile),
SendTabToSelfSyncServiceFactory::GetForProfile(profile),
profile->GetPrefs());
}

bool ShouldDisplayEntryPoint(content::WebContents* web_contents) {
Expand Down
16 changes: 2 additions & 14 deletions chrome/browser/send_tab_to_self/send_tab_to_self_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef CHROME_BROWSER_SEND_TAB_TO_SELF_SEND_TAB_TO_SELF_UTIL_H_
#define CHROME_BROWSER_SEND_TAB_TO_SELF_SEND_TAB_TO_SELF_UTIL_H_

#include "components/send_tab_to_self/entry_point_display_reason.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

namespace content {
Expand All @@ -13,20 +14,7 @@ class WebContents;

namespace send_tab_to_self {

enum class EntryPointDisplayReason {
// The send-tab-to-self entry point should be shown because all the conditions
// are met and the feature is ready to be used.
kOfferFeature,
// The user might be able to use send-tab-to-self if they sign in, so offer
// that. "Might" because the list of target devices can't be known yet, it
// could be empty (see below).
kOfferSignIn,
// All the conditions for send-tab-to-self are met, but there is no valid
// target device. In that case the entry point should inform the user they
// can enjoy the feature by signing in on other devices.
kInformNoTargetDevice,
};

// |web_contents| can be null.
absl::optional<EntryPointDisplayReason> GetEntryPointDisplayReason(
content::WebContents* web_contents);

Expand Down
100 changes: 0 additions & 100 deletions chrome/browser/send_tab_to_self/send_tab_to_self_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ namespace {

const char kHttpsUrl[] = "https://www.foo.com";
const char kHttpsUrl2[] = "https://www.bar.com";
const char kHttpUrl[] = "http://www.foo.com";

class FakeSendTabToSelfModel : public TestSendTabToSelfModel {
public:
Expand Down Expand Up @@ -106,105 +105,6 @@ class SendTabToSelfUtilTest : public BrowserWithTestWindowTest {
}
};

TEST_F(SendTabToSelfUtilTest,
ShouldHideEntryPointIfSignedOutAndPromoFeatureDisabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(kSendTabToSelfSigninPromo);

NavigateAndCommitActiveTab(GURL(kHttpsUrl));
EXPECT_FALSE(GetEntryPointDisplayReason(web_contents()));
}

TEST_F(SendTabToSelfUtilTest,
ShouldShowPromoIfSignedOutAndPromoFeatureEnabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(kSendTabToSelfSigninPromo);

NavigateAndCommitActiveTab(GURL(kHttpsUrl));
EXPECT_EQ(EntryPointDisplayReason::kOfferSignIn,
GetEntryPointDisplayReason(web_contents()));
}

TEST_F(SendTabToSelfUtilTest, ShouldHideEntryPointIfModelNotReady) {
SignIn();
service()->GetSendTabToSelfModel()->SetIsReady(false);
service()->GetSendTabToSelfModel()->SetHasValidTargetDevice(false);

NavigateAndCommitActiveTab(GURL(kHttpsUrl));
EXPECT_FALSE(GetEntryPointDisplayReason(web_contents()));
}

TEST_F(SendTabToSelfUtilTest,
ShouldHideEntryPointIfHasNoValidTargetDeviceAndPromoFeatureDisabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(kSendTabToSelfSigninPromo);

SignIn();
service()->GetSendTabToSelfModel()->SetIsReady(true);
service()->GetSendTabToSelfModel()->SetHasValidTargetDevice(false);

NavigateAndCommitActiveTab(GURL(kHttpsUrl));
EXPECT_FALSE(GetEntryPointDisplayReason(web_contents()));
}

TEST_F(SendTabToSelfUtilTest,
ShouldShowPromoIfHasNoValidTargetDeviceAndPromoFeatureEnabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(kSendTabToSelfSigninPromo);

SignIn();
service()->GetSendTabToSelfModel()->SetIsReady(true);
service()->GetSendTabToSelfModel()->SetHasValidTargetDevice(false);

NavigateAndCommitActiveTab(GURL(kHttpsUrl));
EXPECT_EQ(EntryPointDisplayReason::kInformNoTargetDevice,
GetEntryPointDisplayReason(web_contents()));
}

TEST_F(SendTabToSelfUtilTest, ShouldOnlyOfferFeatureIfHttpOrHttps) {
SignIn();
service()->GetSendTabToSelfModel()->SetIsReady(true);
service()->GetSendTabToSelfModel()->SetHasValidTargetDevice(true);

NavigateAndCommitActiveTab(GURL(kHttpsUrl));
EXPECT_EQ(EntryPointDisplayReason::kOfferFeature,
GetEntryPointDisplayReason(web_contents()));

NavigateAndCommitActiveTab(GURL(kHttpUrl));
EXPECT_EQ(EntryPointDisplayReason::kOfferFeature,
GetEntryPointDisplayReason(web_contents()));

NavigateAndCommitActiveTab(GURL("192.168.0.0"));
EXPECT_FALSE(GetEntryPointDisplayReason(web_contents()));

NavigateAndCommitActiveTab(GURL("chrome-untrusted://url"));
EXPECT_FALSE(GetEntryPointDisplayReason(web_contents()));

NavigateAndCommitActiveTab(GURL("chrome://flags"));
EXPECT_FALSE(GetEntryPointDisplayReason(web_contents()));

NavigateAndCommitActiveTab(GURL("tel:07399999999"));
EXPECT_FALSE(GetEntryPointDisplayReason(web_contents()));
}

TEST_F(SendTabToSelfUtilTest, ShouldHideEntryPointInIncognitoMode) {
// TODO(crbug.com/1313539): This isn't a great way to fake an off-the-record
// profile, but BrowserWithTestWindowTest lacks support. More concretely, this
// harness relies on TestingProfileManager, and the only fitting method there
// is broken (CreateGuestProfile()).
SendTabToSelfSyncServiceFactory::GetInstance()->SetTestingFactory(
profile(),
base::BindRepeating(
[](content::BrowserContext*) -> std::unique_ptr<KeyedService> {
return nullptr;
}));

// Note: if changing this, audit profile-finding logic in the feature.
// For example, NotificationManager.java in the Android code assumes
// incognito is not supported.
EXPECT_FALSE(GetEntryPointDisplayReason(web_contents()));
}

TEST_F(SendTabToSelfUtilTest, ShouldHideEntryPointInOmniboxWhileNavigating) {
SignIn();
service()->GetSendTabToSelfModel()->SetIsReady(true);
Expand Down
8 changes: 8 additions & 0 deletions components/send_tab_to_self/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

static_library("send_tab_to_self") {
sources = [
"entry_point_display_reason.cc",
"entry_point_display_reason.h",
"features.cc",
"features.h",
"metrics_util.cc",
Expand All @@ -28,7 +30,10 @@ static_library("send_tab_to_self") {
"//base",
"//components/history/core/browser",
"//components/keyed_service/core",
"//components/prefs",
"//components/send_tab_to_self/proto:send_tab_to_self_proto",
"//components/signin/public/base",
"//components/signin/public/identity_manager",
"//components/strings",
"//components/sync",
"//components/sync_device_info",
Expand Down Expand Up @@ -65,16 +70,19 @@ source_set("test_support") {
source_set("unit_tests") {
testonly = true
sources = [
"entry_point_display_reason_unittest.cc",
"features_unittest.cc",
"send_tab_to_self_bridge_unittest.cc",
"send_tab_to_self_entry_unittest.cc",
"target_device_info_unittest.cc",
]
deps = [
":send_tab_to_self",
":test_support",
"//base",
"//base/test:test_support",
"//components/history/core/browser",
"//components/prefs:test_support",
"//components/send_tab_to_self/proto:send_tab_to_self_proto",
"//components/sync:test_support",
"//components/sync_device_info:test_support",
Expand Down
3 changes: 3 additions & 0 deletions components/send_tab_to_self/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ include_rules = [
"+components/history",
"+components/infobars",
"+components/keyed_service/core",
"+components/prefs",
"+components/signin/public/base",
"+components/signin/public/identity_manager",
"+components/strings",
"+components/sync",
"+components/sync_device_info",
Expand Down
62 changes: 62 additions & 0 deletions components/send_tab_to_self/entry_point_display_reason.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright 2022 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/send_tab_to_self/entry_point_display_reason.h"

#include "components/prefs/pref_service.h"
#include "components/send_tab_to_self/features.h"
#include "components/send_tab_to_self/send_tab_to_self_model.h"
#include "components/send_tab_to_self/send_tab_to_self_sync_service.h"
#include "components/signin/public/base/signin_pref_names.h"
#include "components/signin/public/identity_manager/account_info.h"
#include "components/sync/driver/sync_service.h"
#include "url/gurl.h"

namespace send_tab_to_self {

namespace {

bool ShouldOfferSignin(syncer::SyncService* sync_service,
PrefService* pref_service) {
return pref_service->GetBoolean(prefs::kSigninAllowed) && sync_service &&
sync_service->GetAccountInfo().IsEmpty() &&
!sync_service->IsLocalSyncEnabled();
}

} // namespace

absl::optional<EntryPointDisplayReason> GetEntryPointDisplayReason(
const GURL& url_to_share,
syncer::SyncService* sync_service,
SendTabToSelfSyncService* send_tab_to_self_sync_service,
PrefService* pref_service) {
if (!url_to_share.SchemeIsHTTPOrHTTPS())
return absl::nullopt;

if (!send_tab_to_self_sync_service) {
// Can happen in incognito or guest profile.
return absl::nullopt;
}

if (ShouldOfferSignin(sync_service, pref_service) &&
base::FeatureList::IsEnabled(kSendTabToSelfSigninPromo)) {
return EntryPointDisplayReason::kOfferSignIn;
}

SendTabToSelfModel* model =
send_tab_to_self_sync_service->GetSendTabToSelfModel();
if (!model->IsReady())
return absl::nullopt;

if (!model->HasValidTargetDevice()) {
return base::FeatureList::IsEnabled(kSendTabToSelfSigninPromo)
? absl::make_optional(
EntryPointDisplayReason::kInformNoTargetDevice)
: absl::nullopt;
}

return EntryPointDisplayReason::kOfferFeature;
}

} // namespace send_tab_to_self
44 changes: 44 additions & 0 deletions components/send_tab_to_self/entry_point_display_reason.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright 2022 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_SEND_TAB_TO_SELF_ENTRY_POINT_DISPLAY_REASON_H_
#define COMPONENTS_SEND_TAB_TO_SELF_ENTRY_POINT_DISPLAY_REASON_H_

#include "third_party/abseil-cpp/absl/types/optional.h"

class GURL;
class PrefService;

namespace syncer {
class SyncService;
} // namespace syncer

namespace send_tab_to_self {

class SendTabToSelfSyncService;

enum class EntryPointDisplayReason {
// The send-tab-to-self entry point should be shown because all the conditions
// are met and the feature is ready to be used.
kOfferFeature,
// The user might be able to use send-tab-to-self if they sign in, so offer
// that. "Might" because the list of target devices can't be known yet, it
// could be empty (see below).
kOfferSignIn,
// All the conditions for send-tab-to-self are met, but there is no valid
// target device. In that case the entry point should inform the user they
// can enjoy the feature by signing in on other devices.
kInformNoTargetDevice,
};

// |sync_service| and |send_tab_to_self_sync_service| can be null.
absl::optional<EntryPointDisplayReason> GetEntryPointDisplayReason(
const GURL& url_to_share,
syncer::SyncService* sync_service,
SendTabToSelfSyncService* send_tab_to_self_sync_service,
PrefService* pref_service);

} // namespace send_tab_to_self

#endif // COMPONENTS_SEND_TAB_TO_SELF_ENTRY_POINT_DISPLAY_REASON_H_

0 comments on commit f2271ab

Please sign in to comment.