Skip to content

Commit

Permalink
Merge pull request #26700 from brave/ksmith-detection-fixes
Browse files Browse the repository at this point in the history
Fix creator detection on reload and youtube channel name detection
  • Loading branch information
zenparsing authored Nov 25, 2024
2 parents e8f5182 + 733fbc9 commit c5b9edd
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 27 deletions.
30 changes: 27 additions & 3 deletions browser/brave_rewards/creator_detection_script_injector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ void CreatorDetectionScriptInjector::MaybeInjectScript(
content::RenderFrameHost* rfh) {
injector_.reset();
injector_host_token_ = content::GlobalRenderFrameHostToken();
last_detection_url_ = GURL();

if (!rfh) {
return;
Expand All @@ -120,17 +121,28 @@ void CreatorDetectionScriptInjector::MaybeInjectScript(
void CreatorDetectionScriptInjector::DetectCreator(
content::RenderFrameHost* rfh,
DetectCreatorCallback callback) {
// Return asynchronously with `nullopt` if `rfh` is invalid or was not
// previously set up via `MaybeInjectScript`, or if the previous call was for
// the same URL.
if (!rfh || rfh->GetGlobalFrameToken() != injector_host_token_ ||
!injector_.is_bound()) {
std::move(callback).Run(std::nullopt);
!injector_.is_bound() ||
rfh->GetLastCommittedURL() == last_detection_url_) {
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&CreatorDetectionScriptInjector::OnDetectionCancelled,
weak_factory_.GetWeakPtr(), std::move(callback)));
return;
}

last_detection_url_ = rfh->GetLastCommittedURL();
++current_request_id_;

// Call the detection function set up by the detection script.
ExecuteScript(
"braveRewards.detectCreator()",
base::BindOnce(&CreatorDetectionScriptInjector::OnCreatorDetected,
weak_factory_.GetWeakPtr(), std::move(callback)));
weak_factory_.GetWeakPtr(), std::move(callback),
current_request_id_));
}

void CreatorDetectionScriptInjector::ExecuteScript(
Expand All @@ -143,9 +155,21 @@ void CreatorDetectionScriptInjector::ExecuteScript(
blink::mojom::PromiseResultOption::kAwait, std::move(callback));
}

void CreatorDetectionScriptInjector::OnDetectionCancelled(
DetectCreatorCallback callback) {
std::move(callback).Run(std::nullopt);
}

void CreatorDetectionScriptInjector::OnCreatorDetected(
DetectCreatorCallback callback,
uint64_t request_id,
base::Value value) {
// Return `nullopt` if this result was for a previous request.
if (request_id != current_request_id_) {
std::move(callback).Run(std::nullopt);
return;
}

Result result;
if (auto* dict = value.GetIfDict()) {
if (auto* id = dict->FindString("id")) {
Expand Down
13 changes: 10 additions & 3 deletions browser/brave_rewards/creator_detection_script_injector.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "brave/components/script_injector/common/mojom/script_injector.mojom.h"
#include "content/public/browser/global_routing_id.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "url/gurl.h"

namespace content {
class RenderFrameHost;
Expand Down Expand Up @@ -55,10 +56,10 @@ class CreatorDetectionScriptInjector {
using DetectCreatorCallback = base::OnceCallback<void(std::optional<Result>)>;

// Runs the creator detection routine initialized by `MaybeInjectScript` and
// asynchrounously returns the detection result. Return `nullopt` if the
// asynchrounously returns the detection result. Returns `nullopt` if the
// detection routine was not invoked (e.g. because Rewards is not enabled or
// because there is no script for this page). Returns a `Result` with empty
// fields if there is not creator associated with the current page. Note that
// fields if there is no creator associated with the current page. Note that
// any of the `Result` fields may be empty if the detection script was unable
// to gather that information from the page.
void DetectCreator(content::RenderFrameHost* rfh,
Expand All @@ -69,10 +70,16 @@ class CreatorDetectionScriptInjector {

void ExecuteScript(std::string_view script, ExecuteScriptCallback callback);

void OnCreatorDetected(DetectCreatorCallback callback, base::Value value);
void OnDetectionCancelled(DetectCreatorCallback callback);

void OnCreatorDetected(DetectCreatorCallback callback,
uint64_t request_id,
base::Value value);

mojo::AssociatedRemote<script_injector::mojom::ScriptInjector> injector_;
content::GlobalRenderFrameHostToken injector_host_token_;
GURL last_detection_url_;
uint64_t current_request_id_ = 0;
base::WeakPtrFactory<CreatorDetectionScriptInjector> weak_factory_{this};
};

Expand Down
24 changes: 6 additions & 18 deletions browser/brave_rewards/rewards_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,30 +112,19 @@ void RewardsTabHelper::DidFinishNavigation(
return;
}

auto& url = navigation_handle->GetURL();

if (!navigation_handle->IsSameDocument()) {
auto id = GetPublisherIdFromURL(url);
auto id = GetPublisherIdFromURL(navigation_handle->GetURL());
SetPublisherIdForTab(id ? *id : "");
MaybeSavePublisherInfo();
rewards_service_->OnUnload(tab_id_);
creator_detection_.MaybeInjectScript(
navigation_handle->GetRenderFrameHost());
}

// Only invoke creator detection if the URL has changed.
if (url != last_detection_url_) {
last_detection_url_ = url;

// Store the current navigation ID so that we can determine if the async
// result corresponds to this navigation.
detection_navigation_id_ = navigation_handle->GetNavigationId();

creator_detection_.DetectCreator(
navigation_handle->GetRenderFrameHost(),
base::BindOnce(&RewardsTabHelper::OnCreatorDetected,
base::Unretained(this), detection_navigation_id_));
}
creator_detection_.DetectCreator(
navigation_handle->GetRenderFrameHost(),
base::BindOnce(&RewardsTabHelper::OnCreatorDetected,
base::Unretained(this)));
}

void RewardsTabHelper::ResourceLoadComplete(
Expand Down Expand Up @@ -224,9 +213,8 @@ void RewardsTabHelper::MaybeSavePublisherInfo() {
}

void RewardsTabHelper::OnCreatorDetected(
int64_t navigation_id,
std::optional<CreatorDetectionScriptInjector::Result> result) {
if (!result || navigation_id != detection_navigation_id_) {
if (!result) {
return;
}

Expand Down
3 changes: 0 additions & 3 deletions browser/brave_rewards/rewards_tab_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ class RewardsTabHelper : public content::WebContentsUserData<RewardsTabHelper>,
void MaybeSavePublisherInfo();

void OnCreatorDetected(
int64_t navigation_id,
std::optional<CreatorDetectionScriptInjector::Result> result);

#if !BUILDFLAG(IS_ANDROID)
Expand All @@ -101,8 +100,6 @@ class RewardsTabHelper : public content::WebContentsUserData<RewardsTabHelper>,
raw_ptr<RewardsService> rewards_service_ = nullptr;
base::ObserverList<Observer> observer_list_;
std::string publisher_id_;
GURL last_detection_url_;
int64_t detection_navigation_id_ = 0;
CreatorDetectionScriptInjector creator_detection_;

WEB_CONTENTS_USER_DATA_KEY_DECL();
Expand Down
12 changes: 12 additions & 0 deletions browser/brave_rewards/test/creator_detection_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,18 @@ IN_PROC_BROWSER_TEST_F(CreatorDetectionBrowserTest, YouTubeDetection) {
EXPECT_EQ(future.Get(), "youtube#channel:987654321");
}

{
// Page reload.
TestFuture<std::string> future;
TabHelperObserver observer(GetRewardsTabHelper(), future.GetCallback());
auto* web_contents = chrome_test_utils::GetActiveWebContents(this);
web_contents->GetController().Reload(content::ReloadType::NORMAL, true);
EXPECT_TRUE(WaitForLoadStop(web_contents));
WaitForTimeout();
EXPECT_EQ(GetRewardsTabHelper().GetPublisherIdForTab(),
"youtube#channel:987654321");
}

{
// Same-document navigation via `history.pushState`.
TestFuture<std::string> future;
Expand Down
10 changes: 10 additions & 0 deletions components/brave_rewards/resources/creator_detection/youtube.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,16 @@ initializeDetector(() => {
}
}

if (currentPathType === 'channel' || currentPathType === 'channel-name') {
let header = document.querySelector<HTMLElement>('#page-header')
if (header) {
let matches = header.innerText.match(/@[\w]+/)
if (matches && matches.length > 0) {
return matches[0]
}
}
}

return ''
}

Expand Down

0 comments on commit c5b9edd

Please sign in to comment.