Skip to content

Commit

Permalink
[CSC] Added support for reporting URL
Browse files Browse the repository at this point in the history
This CL adds support for opening reporting URL on a new tab.

(cherry picked from commit aba062b)

Change-Id: I8aba18d19bb6f5988a35363377c751d03382089a
Bug: b/283972294, crbug/1449021
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4555990
Reviewed-by: Alex Gough <ajgo@chromium.org>
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1148209}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4582230
Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Cr-Commit-Position: refs/branch-heads/5790@{#240}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Shakti Sahu authored and Chromium LUCI CQ committed Jun 1, 2023
1 parent f999a78 commit 1a61c8c
Show file tree
Hide file tree
Showing 12 changed files with 69 additions and 14 deletions.
Expand Up @@ -33,7 +33,7 @@ class MockSigninDelegate : public SigninDelegate {
MOCK_METHOD0(IsSignedIn, bool());
MOCK_METHOD0(StartSigninFlow, void());
MOCK_METHOD1(EnableMsbb, void(bool));
MOCK_METHOD1(LoadExpsPromUrl, void(const GURL&));
MOCK_METHOD1(LoadUrlInNewTab, void(const GURL&));
};

} // namespace
Expand Down
7 changes: 6 additions & 1 deletion chrome/browser/companion/core/mojom/companion.mojom
Expand Up @@ -84,6 +84,9 @@ enum PhFeedback {

// User clicked thumbs down.
kThumbsDown = 2,

// User clicked report content.
kReportContent = 3,
};

// Data for uploading an image query.
Expand Down Expand Up @@ -178,7 +181,9 @@ interface CompanionPageHandler {
OnCqCandidatesAvailable(array<string> text_directives);

// Called in response to user feedback action on the PH surface.
OnPhFeedback(PhFeedback ph_feedback);
// `reporting_url` is optionally used to pass the URL to use for reporting
// content.
OnPhFeedback(PhFeedback ph_feedback, url.mojom.Url? reporting_url);

// Called to notify the browser that the user has clicked on a jumptag.
// `text_directive` is the URL fragment for highlighting and scrolling-to.
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/companion/core/promo_handler.cc
Expand Up @@ -74,7 +74,7 @@ void PromoHandler::OnExpsPromo(PromoAction promo_action,
IncrementPref(kExpsPromoDeclinedCountPref);
} else if (promo_action == PromoAction::kAccepted) {
if (exps_promo_url.has_value()) {
signin_delegate_->LoadExpsPromUrl(exps_promo_url.value());
signin_delegate_->LoadUrlInNewTab(exps_promo_url.value());
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/companion/core/promo_handler_unittest.cc
Expand Up @@ -22,7 +22,7 @@ class MockSigninDelegate : public SigninDelegate {
MOCK_METHOD0(IsSignedIn, bool());
MOCK_METHOD0(StartSigninFlow, void());
MOCK_METHOD1(EnableMsbb, void(bool));
MOCK_METHOD1(LoadExpsPromUrl, void(const GURL&));
MOCK_METHOD1(LoadUrlInNewTab, void(const GURL&));
};

} // namespace
Expand Down Expand Up @@ -73,11 +73,11 @@ TEST_F(PromoHandlerTest, ExpsPromo) {
absl::nullopt);
EXPECT_EQ(1, pref_service_.GetInteger(kExpsPromoDeclinedCountPref));

EXPECT_CALL(signin_delegate_, LoadExpsPromUrl(testing::_)).Times(0);
EXPECT_CALL(signin_delegate_, LoadUrlInNewTab(testing::_)).Times(0);
promo_handler_->OnPromoAction(PromoType::kExps, PromoAction::kAccepted,
absl::nullopt);

EXPECT_CALL(signin_delegate_, LoadExpsPromUrl(testing::_)).Times(1);
EXPECT_CALL(signin_delegate_, LoadUrlInNewTab(testing::_)).Times(1);
promo_handler_->OnPromoAction(PromoType::kExps, PromoAction::kAccepted,
GURL());
}
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/companion/core/signin_delegate.h
Expand Up @@ -32,8 +32,8 @@ class SigninDelegate {
// Enable the setting for make searches and browsing better.
virtual void EnableMsbb(bool enable_msbb) = 0;

// Loads exps promo URL in the browser.
virtual void LoadExpsPromUrl(const GURL& exps_promo_url) = 0;
// Loads URL in the browser in a new tab.
virtual void LoadUrlInNewTab(const GURL& url) = 0;
};

} // namespace companion
Expand Down
6 changes: 5 additions & 1 deletion chrome/browser/resources/side_panel/companion/companion.ts
Expand Up @@ -36,6 +36,7 @@ enum ParamType {

// Arguments for MethodType.kOnPhFeedback.
PH_FEEDBACK = 'phFeedback',
REPORTING_URL = 'reportingUrl',

// Arguments for MethodType.kOnOpenInNewTabButtonURLChanged.
URL_FOR_OPEN_IN_NEW_TAB = 'urlForOpenInNewTab',
Expand Down Expand Up @@ -214,7 +215,10 @@ function onCompanionMessageEvent(event: MessageEvent) {
companionProxy.handler.onCqCandidatesAvailable(
data[ParamType.CQ_TEXT_DIRECTIVES]);
} else if (methodType === MethodType.kOnPhFeedback) {
companionProxy.handler.onPhFeedback(data[ParamType.PH_FEEDBACK]);
const reportingUrl = new Url();
reportingUrl.url = data[ParamType.REPORTING_URL] || '';
companionProxy.handler.onPhFeedback(
data[ParamType.PH_FEEDBACK], reportingUrl);
} else if (methodType === MethodType.kOnCqJumptagClicked) {
companionProxy.handler.onCqJumptagClicked(data[ParamType.CQ_JUMPTAG_TEXT]);
}
Expand Down
Expand Up @@ -70,6 +70,7 @@ const char kHost[] = "foo.com";
const char kSearchQueryUrl[] = "https://www.google.com/search?q=xyz";

const char kExpectedExpsPromoUrl[] = "https://foobar.com/";
const char kPhReportingUrl[] = "https://foobar.com/";

} // namespace

Expand All @@ -84,6 +85,8 @@ struct CompanionScriptBuilder {
absl::optional<PromoType> promo_type;
absl::optional<PromoAction> promo_action;
absl::optional<std::string> exps_promo_url;
absl::optional<PhFeedback> ph_feedback;
absl::optional<std::string> reporting_url;
absl::optional<bool> is_exps_opted_in;
absl::optional<UiSurface> ui_surface;
absl::optional<int> ui_surface_position;
Expand Down Expand Up @@ -124,6 +127,16 @@ struct CompanionScriptBuilder {
ss << "message['expsPromoUrl'] = '" << exps_promo_url.value() << "';";
}

if (ph_feedback.has_value()) {
ss << "message['phFeedback'] = "
<< base::NumberToString(static_cast<size_t>(ph_feedback.value()))
<< ";";
}

if (reporting_url.has_value()) {
ss << "message['reportingUrl'] = '" << reporting_url.value() << "';";
}

if (is_exps_opted_in.has_value()) {
ss << "message['isExpsOptedIn'] = "
<< base::NumberToString(is_exps_opted_in.value()) << ";";
Expand Down Expand Up @@ -815,6 +828,35 @@ IN_PROC_BROWSER_TEST_F(CompanionPageBrowserTest, RegionSearchClick) {
ukm::builders::Companion_PageView::kRegionSearch_ClickCountName, 1);
}

IN_PROC_BROWSER_TEST_F(CompanionPageBrowserTest, PhFeedbackWithReportContent) {
ukm::TestAutoSetUkmRecorder ukm_recorder;
// Load a page on the active tab.
ASSERT_TRUE(
ui_test_utils::NavigateToURL(browser(), CreateUrl(kHost, kRelativeUrl1)));
ASSERT_EQ(side_panel_coordinator()->GetCurrentEntryId(), absl::nullopt);

EXPECT_EQ(1, browser()->tab_strip_model()->count());

// Open companion companion via toolbar entry point.
side_panel_coordinator()->Show(SidePanelEntry::Id::kSearchCompanion);
EXPECT_TRUE(side_panel_coordinator()->IsSidePanelShowing());

WaitForCompanionToBeLoaded();
EXPECT_EQ(side_panel_coordinator()->GetCurrentEntryId(),
SidePanelEntry::Id::kSearchCompanion);

// Show exps promo, user accepts it.
CompanionScriptBuilder builder(MethodType::kOnPhFeedback);
builder.ph_feedback = PhFeedback::kReportContent;
builder.reporting_url = kPhReportingUrl;
EXPECT_TRUE(ExecJs(builder.Build()));

// Verify that a new tab opens up to load the exps URL.
WaitForTabCount(2);
EXPECT_TRUE(
web_contents()->GetVisibleURL().spec().starts_with(kPhReportingUrl));
}

IN_PROC_BROWSER_TEST_F(CompanionPageBrowserTest,
PostMessageForCqCandidatesAvailable) {
// Load a page on the active tab.
Expand Down
Expand Up @@ -246,7 +246,9 @@ void CompanionPageHandler::OnCqCandidatesAvailable(
}

void CompanionPageHandler::OnPhFeedback(
side_panel::mojom::PhFeedback ph_feedback) {
side_panel::mojom::PhFeedback ph_feedback,
const absl::optional<GURL>& reporting_url) {
signin_delegate_->LoadUrlInNewTab(reporting_url.value());
metrics_logger_->OnPhFeedback(ph_feedback);
}

Expand Down
Expand Up @@ -58,7 +58,8 @@ class CompanionPageHandler
int32_t click_position) override;
void OnCqCandidatesAvailable(
const std::vector<std::string>& text_directives) override;
void OnPhFeedback(side_panel::mojom::PhFeedback ph_feedback) override;
void OnPhFeedback(side_panel::mojom::PhFeedback ph_feedback,
const absl::optional<GURL>& reporting_url) override;
void OnCqJumptagClicked(const std::string& text_directive) override;

// content::WebContentsObserver overrides.
Expand Down
Expand Up @@ -66,8 +66,8 @@ void SigninDelegateImpl::EnableMsbb(bool enable_msbb) {
consent_service->SetUrlKeyedAnonymizedDataCollectionEnabled(enable_msbb);
}

void SigninDelegateImpl::LoadExpsPromUrl(const GURL& exps_promo_url) {
content::OpenURLParams params(exps_promo_url, content::Referrer(),
void SigninDelegateImpl::LoadUrlInNewTab(const GURL& url) {
content::OpenURLParams params(url, content::Referrer(),
WindowOpenDisposition::NEW_FOREGROUND_TAB,
ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
/*is_renderer_initiated*/ false);
Expand Down
Expand Up @@ -30,7 +30,7 @@ class SigninDelegateImpl : public SigninDelegate {
bool IsSignedIn() override;
void StartSigninFlow() override;
void EnableMsbb(bool enable_msbb) override;
void LoadExpsPromUrl(const GURL& exps_promo_url) override;
void LoadUrlInNewTab(const GURL& url) override;

private:
Profile* GetProfile();
Expand Down
1 change: 1 addition & 0 deletions tools/metrics/histograms/enums.xml
Expand Up @@ -18244,6 +18244,7 @@ Called by update_net_error_codes.py.-->
<enum name="Companion.PhFeedback">
<int value="1" label="Thumbs up"/>
<int value="2" label="Thumbs down"/>
<int value="3" label="Report content"/>
</enum>

<enum name="Companion.PromoEvent">
Expand Down

0 comments on commit 1a61c8c

Please sign in to comment.