Skip to content

Commit

Permalink
optimization: don't parse yt metadata (or fetch transcript) until an …
Browse files Browse the repository at this point in the history
…ai chat message is sent by the user
  • Loading branch information
petemill committed Feb 28, 2024
1 parent 2099cb9 commit a968320
Show file tree
Hide file tree
Showing 15 changed files with 204 additions and 193 deletions.
36 changes: 3 additions & 33 deletions components/ai_chat/content/browser/ai_chat_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,6 @@ void AIChatTabHelper::DidFinishNavigation(
is_same_document_navigation_ = navigation_handle->IsSameDocument();
pending_navigation_id_ = navigation_handle->GetNavigationId();

// Re-assess whether we get content from subresource interception on every
// navigation (same-document or not).
SetIsContentSubresourceDependent(false);

// Experimentally only call |OnNewPage| for same-page navigations _if_
// it results in a page title change (see |TtileWasSet|).
if (!is_same_document_navigation_) {
Expand Down Expand Up @@ -219,23 +215,11 @@ void AIChatTabHelper::OnFaviconUpdated(
}

// mojom::PageContentExtractorHost
void AIChatTabHelper::OnInterceptedPageContentChanged(
mojom::PageContentPtr content) {
void AIChatTabHelper::OnInterceptedPageContentChanged() {
DVLOG(2) << __func__;

// TODO(petemill): Only accept basic content string - no fetching or even json
// parsing. Store in local variable until getpagecontent is called. This is
// performance optimization so that we're not doing extra work when there's no
// active conversation. We still need to store this data for inactive
// conversations so that if a conversation is opened for the current
// navigation then we are able to retrieve the content that's only accessible
// via subresource interception.

// Stop reading page content from document, use subresource instead from now
// on.
SetIsContentSubresourceDependent(true);
// Maybe mark that the page changed, if we didn't detect it already via title
// change after a same-page navigation.
// change after a same-page navigation. This is the main benefit of this
// function.
if (is_same_document_navigation_) {
DVLOG(2) << "Same document navigation detected new \"page\" - calling "
"OnNewPage()";
Expand All @@ -245,20 +229,6 @@ void AIChatTabHelper::OnInterceptedPageContentChanged(
// Don't respond to further TitleWasSet
is_same_document_navigation_ = false;
}
// Fetch new page text and update the conversation
// TODO(petemill): use method with WeakPtr because FetchPageContent might hold
// on to callback after TabHelper destroyed.
GetPageContentCallback cb = base::BindOnce(
[](AIChatTabHelper* instance, const int64_t navigation_id,
std::string content, bool success, std::string invalidation_token) {
if (instance->pending_navigation_id_ != navigation_id) {
DVLOG(1) << "Ignoring content update for stale navigation id";
return;
}
instance->OnPageContentUpdated(content, success, invalidation_token);
},
base::Unretained(this), pending_navigation_id_);
FetchPageContent(web_contents(), std::move(content), std::move(cb));
}

// ai_chat::ConversationDriver
Expand Down
3 changes: 1 addition & 2 deletions components/ai_chat/content/browser/ai_chat_tab_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ class AIChatTabHelper : public content::WebContentsObserver,
void SetOnPDFA11yInfoLoadedCallbackForTesting(base::OnceClosure cb);

// mojom::PageContentExtractorHost
void OnInterceptedPageContentChanged(
mojom::PageContentPtr page_content) override;
void OnInterceptedPageContentChanged() override;

private:
friend class content::WebContentsUserData<AIChatTabHelper>;
Expand Down
13 changes: 0 additions & 13 deletions components/ai_chat/content/browser/page_content_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -523,17 +523,4 @@ void FetchPageContent(content::WebContents* web_contents,
fetcher->Start(std::move(extractor), invalidation_token, std::move(callback));
}

void FetchPageContent(content::WebContents* web_contents,
mojom::PageContentPtr content_hint,
FetchPageContentCallback callback) {
auto* loader = web_contents->GetBrowserContext()
->GetDefaultStoragePartition()
->GetURLLoaderFactoryForBrowserProcess()
.get();
auto* fetcher = new PageContentFetcher(loader);
// Invalidation token is blank because having PageContent implies content is
// new.
fetcher->OnTabContentResult(std::move(callback), "", std::move(content_hint));
}

} // namespace ai_chat
3 changes: 0 additions & 3 deletions components/ai_chat/content/browser/page_content_fetcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ void FetchPageContent(content::WebContents* web_contents,
FetchPageContentCallback callback,
scoped_refptr<network::SharedURLLoaderFactory>
url_loader_factory = nullptr);
void FetchPageContent(content::WebContents* web_contents,
mojom::PageContentPtr content_hint,
FetchPageContentCallback callback);

} // namespace ai_chat

Expand Down
8 changes: 0 additions & 8 deletions components/ai_chat/core/browser/conversation_driver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -426,14 +426,6 @@ void ConversationDriver::GeneratePageContent(GetPageContentCallback callback) {
DCHECK(is_conversation_active_)
<< "UI shouldn't allow operations for an inactive conversation";

if (is_content_subresource_dependent_ && !article_text_.empty()) {
// If the content is pushed from renderer, then we possibly do not have
// a way to fetch it on-demand. So we provide the cached content instead.
std::move(callback).Run(article_text_, is_video_,
content_invalidation_token_);
return;
}

// Only perform a fetch once at a time, and then use the results from
// an in-progress operation.
if (is_page_text_fetch_in_progress_) {
Expand Down
7 changes: 0 additions & 7 deletions components/ai_chat/core/browser/conversation_driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,6 @@ class ConversationDriver {
// is expected.
void OnNewPage(int64_t navigation_id);

// Set the flag to indicate that the page content can not be fetched on-demand
// and is dependent on intercepted subresources.
void SetIsContentSubresourceDependent(bool is_content_subresource_dependent) {
is_content_subresource_dependent_ = is_content_subresource_dependent;
}

private:
void InitEngine();
void OnUserOptedIn();
Expand Down Expand Up @@ -223,7 +217,6 @@ class ConversationDriver {
std::string article_text_;
std::string content_invalidation_token_;
bool is_page_text_fetch_in_progress_ = false;
bool is_content_subresource_dependent_ = false;
std::unique_ptr<base::OneShotEvent> on_page_text_fetch_complete_;

bool is_request_in_progress_ = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,8 @@ interface PageContentExtractor {
// Allows the renderer to notify the browser process of meaningful changes to
// the content.
interface PageContentExtractorHost {
OnInterceptedPageContentChanged(PageContent page_content);
// We don't send page content here due to an optimization for the majority of
// renderers without active AIChat conversations. We wait until the host
// requests it via PageContentExtractor.ExtractPageContent.
OnInterceptedPageContentChanged();
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,22 @@
#ifndef BRAVE_COMPONENTS_AI_CHAT_RENDERER_AI_CHAT_RESOURCE_SNIFFER_THROTTLE_DELEGATE_H_
#define BRAVE_COMPONENTS_AI_CHAT_RENDERER_AI_CHAT_RESOURCE_SNIFFER_THROTTLE_DELEGATE_H_

#include "brave/components/ai_chat/core/common/mojom/page_content_extractor.mojom.h"
#include <memory>
#include <string>

namespace ai_chat {

class AIChatResourceSnifferThrottleDelegate {
public:
enum class InterceptedContentType {
kYouTubeMetadataString,
};
struct InterceptedContent {
InterceptedContentType type;
std::string content;
};
virtual void OnInterceptedPageContentChanged(
mojom::PageContentPtr content) = 0;
std::unique_ptr<InterceptedContent> content) = 0;

protected:
virtual ~AIChatResourceSnifferThrottleDelegate() = default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include "services/network/public/mojom/url_response_head.mojom.h"
#include "services/network/test/test_url_loader_client.h"
#include "services/network/test/test_url_loader_factory.h"
#include "testing/gmock/include/gmock/gmock-matchers.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/loader/url_loader_throttle.h"
#include "third_party/googletest/src/googletest/include/gtest/gtest.h"
Expand Down Expand Up @@ -89,18 +89,17 @@ class MojoDataPipeSender {
class MockAIChatResourceSnifferThrottleDelegate
: public AIChatResourceSnifferThrottleDelegate {
public:
MockAIChatResourceSnifferThrottleDelegate() = default;
~MockAIChatResourceSnifferThrottleDelegate() override = default;

void OnInterceptedPageContentChanged(mojom::PageContentPtr content) override {
ASSERT_TRUE(content->content->is_content_url());
ASSERT_EQ(content->type, mojom::PageContentType::VideoTranscriptYouTube);
OnInterceptedPageContentChanged_Data(
content->content->get_content_url().spec());
}

MOCK_METHOD(void, OnInterceptedPageContentChanged_Data, (std::string));

void OnInterceptedPageContentChanged(
std::unique_ptr<AIChatResourceSnifferThrottleDelegate::InterceptedContent>
content) override {
ASSERT_EQ(content->type,
AIChatResourceSnifferThrottleDelegate::InterceptedContentType::
kYouTubeMetadataString);
OnInterceptedPageContentChanged_Data(content->content);
}

base::WeakPtrFactory<MockAIChatResourceSnifferThrottleDelegate> weak_factory_{
this};
};
Expand Down Expand Up @@ -281,30 +280,19 @@ TEST_F(AIChatResourceSnifferThrottleTest, DoesNotThrottleNonHTTP) {
}

TEST_F(AIChatResourceSnifferThrottleTest, Body_NonJson) {
// PNG
InterceptBodyRequestFor("\x89PNG\x0D\x0A\x1A\x0A");
EXPECT_CALL(ai_chat_throttle_delegate_, OnInterceptedPageContentChanged_Data)
.Times(0);
}

TEST_F(AIChatResourceSnifferThrottleTest, Body_InvalidJson) {
InterceptBodyRequestFor("{");
EXPECT_CALL(ai_chat_throttle_delegate_, OnInterceptedPageContentChanged_Data)
.Times(0);
}

TEST_F(AIChatResourceSnifferThrottleTest, Body_ValidNonYTJson) {
InterceptBodyRequestFor(R"({"captions": []})");
EXPECT_CALL(ai_chat_throttle_delegate_, OnInterceptedPageContentChanged_Data)
.Times(0);
// AIChatResourceSnifferThrottle doesn't parse the json as an optimization
// since it might not get used until an AIChat conversation message is about
// to be sent, so any body content should be passed to the delegate, we don't
// need to test for valid JSON
std::string body = "\x89PNG\x0D\x0A\x1A\x0A";
EXPECT_CALL(ai_chat_throttle_delegate_,
OnInterceptedPageContentChanged_Data(body))
.Times(1);
InterceptBodyRequestFor(body);
}

TEST_F(AIChatResourceSnifferThrottleTest, Body_ValidYTJson) {
EXPECT_CALL(
ai_chat_throttle_delegate_,
OnInterceptedPageContentChanged_Data("https://www.example.com/caption1"))
.Times(1);
InterceptBodyRequestFor(R"({
std::string body = R"({
"captions": {
"playerCaptionsTracklistRenderer": {
"captionTracks": [
Expand All @@ -314,37 +302,20 @@ TEST_F(AIChatResourceSnifferThrottleTest, Body_ValidYTJson) {
]
}
}
})");
mojom::PageContentPtr expected_content = mojom::PageContent::New();
expected_content->type = mojom::PageContentType::VideoTranscriptYouTube;
expected_content->content = mojom::PageContentData::NewContentUrl(GURL());
})";
EXPECT_CALL(ai_chat_throttle_delegate_,
OnInterceptedPageContentChanged_Data(body))
.Times(1);
InterceptBodyRequestFor(body);
}

TEST_F(AIChatResourceSnifferThrottleTest, Abort_NoBodyPipe) {
GURL url("https://www.youtube.com/youtubei/v1/player");
auto throttle = MaybeCreateThrottleForUrl(url);
auto delegate = std::make_unique<MockDelegate>();
throttle->set_delegate(delegate.get());

auto response_head = network::mojom::URLResponseHead::New();
bool defer = false;
throttle->WillProcessResponse(url, response_head.get(), &defer);
EXPECT_FALSE(defer);
EXPECT_TRUE(delegate->is_intercepted());

// Send the body
std::string body = "This should be long enough to complete sniffing.";
body.resize(1024, 'a');
delegate->LoadResponseBody(body);
task_environment_.RunUntilIdle();

// Release a pipe for the body on the receiver side.
delegate->destination_loader_client()->response_body_release();
task_environment_.RunUntilIdle();

// Calling OnComplete should not crash.
delegate->CompleteResponse();
task_environment_.RunUntilIdle();
TEST_F(AIChatResourceSnifferThrottleTest, LongBody) {
std::string body = "This should be long enough...";
body.resize(2048, 'a');
EXPECT_CALL(ai_chat_throttle_delegate_,
OnInterceptedPageContentChanged_Data(body))
.Times(1);
InterceptBodyRequestFor(body);
}

} // namespace ai_chat
49 changes: 8 additions & 41 deletions components/ai_chat/renderer/ai_chat_resource_sniffer_url_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@

#include "brave/components/ai_chat/renderer/ai_chat_resource_sniffer_url_loader.h"

#include <memory>
#include <string>
#include <utility>

#include "base/json/json_reader.h"
#include "base/logging.h"
#include "base/types/expected.h"
#include "base/values.h"
#include "brave/components/ai_chat/core/common/mojom/page_content_extractor.mojom.h"
#include "brave/components/ai_chat/renderer/yt_util.h"
#include "brave/components/body_sniffer/body_sniffer_url_loader.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h"

Expand All @@ -23,32 +22,6 @@ namespace {

constexpr uint32_t kReadBufferSize = 37000; // average subresource size

std::optional<std::string> GetCaptionTrackUrl(std::string& body) {
if (!body.size()) {
return std::nullopt;
}

auto result_value =
base::JSONReader::ReadAndReturnValueWithError(body, base::JSON_PARSE_RFC);

if (!result_value.has_value() || result_value->is_string()) {
VLOG(1) << __func__ << ": parsing error: " << result_value.ToString();
return std::nullopt;
} else if (!result_value->is_dict()) {
VLOG(1) << __func__ << ": parsing error: not a dict";
return std::nullopt;
}

auto* caption_tracks = result_value->GetDict().FindListByDottedPath(
"captions.playerCaptionsTracklistRenderer.captionTracks");
if (!caption_tracks) {
VLOG(1) << __func__ << ": no caption tracks found";
return std::nullopt;
}

return ChooseCaptionTrackUrl(caption_tracks);
}

} // namespace

// static
Expand Down Expand Up @@ -115,20 +88,14 @@ void AIChatResourceSnifferURLLoader::OnBodyWritable(MojoResult r) {
void AIChatResourceSnifferURLLoader::CompleteLoading(std::string body) {
DVLOG(4) << __func__ << ": got body length: " << body.size()
<< " for url: " << response_url_.spec();

auto maybe_caption_url = GetCaptionTrackUrl(body);

if (maybe_caption_url.has_value()) {
GURL caption_url = response_url_.Resolve(maybe_caption_url.value());
if (caption_url.is_valid()) {
mojom::PageContentPtr content_update = mojom::PageContent::New();
content_update->type = mojom::PageContentType::VideoTranscriptYouTube;
content_update->content =
mojom::PageContentData::NewContentUrl(GURL(caption_url));
delegate_->OnInterceptedPageContentChanged(std::move(content_update));
}
if (!body.empty()) {
auto content = std::make_unique<
AIChatResourceSnifferThrottleDelegate::InterceptedContent>();
content->type = AIChatResourceSnifferThrottleDelegate::
InterceptedContentType::kYouTubeMetadataString;
content->content = body;
delegate_->OnInterceptedPageContentChanged(std::move(content));
}

body_sniffer::BodySnifferURLLoader::CompleteLoading(std::move(body));
}

Expand Down
Loading

0 comments on commit a968320

Please sign in to comment.