review feedback

components/ai_chat/content/browser/ai_chat_tab_helper.cc

@@ -67,7 +67,7 @@ AIChatTabHelper::PDFA11yInfoLoadObserver::~PDFA11yInfoLoadObserver() = default;
 void AIChatTabHelper::BindPageContentExtractorHost(
     content::RenderFrameHost* rfh,
     mojo::PendingAssociatedReceiver<mojom::PageContentExtractorHost> receiver) {
-  DCHECK(rfh);
+  CHECK(rfh);
   if (!rfh->IsInPrimaryMainFrame()) {
     DVLOG(4) << "Not binding extractor host to non-main frame";
     return;
@@ -221,7 +221,6 @@ void AIChatTabHelper::OnFaviconUpdated(
 
 // mojom::PageContentExtractorHost
 void AIChatTabHelper::OnInterceptedPageContentChanged() {
-  DVLOG(2) << __func__;
   // Maybe mark that the page changed, if we didn't detect it already via title
   // change after a same-page navigation. This is the main benefit of this
   // function.

components/ai_chat/core/browser/conversation_driver.cc

@@ -472,26 +472,24 @@ void ConversationDriver::OnGeneratePageContentComplete(
     std::string contents_text,
     bool is_video,
     std::string invalidation_token) {
-  VLOG(1) << "OnGeneratePageContentComplete";
-  VLOG(4) << "Contents(is_video=" << is_video
-          << ", invalidation_token=" << invalidation_token
-          << "): " << contents_text;
+  DVLOG(1) << "OnGeneratePageContentComplete";
+  DVLOG(4) << "Contents(is_video=" << is_video
+           << ", invalidation_token=" << invalidation_token
+           << "): " << contents_text;
   if (navigation_id != current_navigation_id_) {
     VLOG(1) << __func__ << " for a different navigation. Ignoring.";
     return;
   }
 
   // Ignore if we received content from observer in the meantime
   if (!is_page_text_fetch_in_progress_) {
-    VLOG(1) << __func__
-            << " but already received contents from observer. Ignoring.";
+    DVLOG(1) << __func__
+             << " but already received contents from observer. Ignoring.";
     return;
   }
 
   OnPageContentUpdated(contents_text, is_video, invalidation_token);
 
-  VLOG(4) << "calling callback with text: " << article_text_;
-
   std::move(callback).Run(article_text_, is_video_,
                           content_invalidation_token_);
 }
@@ -500,8 +498,8 @@ void ConversationDriver::OnExistingGeneratePageContentComplete(
     GetPageContentCallback callback) {
   // Don't need to check navigation ID since existing event will be
   // deleted when there's a new conversation.
-  VLOG(1) << "Existing page content fetch completed, proceeding with "
-             "the results of that operation.";
+  DVLOG(1) << "Existing page content fetch completed, proceeding with "
+              "the results of that operation.";
   std::move(callback).Run(article_text_, is_video_,
                           content_invalidation_token_);
 }

components/ai_chat/core/browser/conversation_driver.h

@@ -147,7 +147,7 @@ class ConversationDriver {
   virtual void OnFaviconImageDataChanged();
 
   // Implementer should call this when the content is updated in a way that
-  // will not be detected by the on-demand techniquesused by GetPageContent.
+  // will not be detected by the on-demand techniques used by GetPageContent.
   // For example for sites where GetPageContent does not read the live DOM but
   // reads static JS from HTML that doesn't change for same-page navigation and
   // we need to intercept new JS data from subresource loads.

components/ai_chat/renderer/ai_chat_resource_sniffer_throttle.cc

@@ -29,7 +29,6 @@ AIChatResourceSnifferThrottle::MaybeCreateThrottleFor(
   // |mojom::PageContent|.
   if (url.SchemeIsHTTPOrHTTPS() && base::Contains(kYouTubeHosts, url.host()) &&
       base::EqualsCaseInsensitiveASCII(url.path(), kYouTubePlayerAPIPath)) {
-    VLOG(1) << __func__ << " Creating throttle for url: " << url.spec();
     return std::make_unique<AIChatResourceSnifferThrottle>(task_runner,
                                                            delegate);
   }

components/ai_chat/renderer/ai_chat_resource_sniffer_url_loader.cc

@@ -86,8 +86,6 @@ void AIChatResourceSnifferURLLoader::OnBodyWritable(MojoResult r) {
 }
 
 void AIChatResourceSnifferURLLoader::CompleteLoading(std::string body) {
-  DVLOG(4) << __func__ << ": got body length: " << body.size()
-           << " for url: " << response_url_.spec();
   if (!body.empty()) {
     auto content = std::make_unique<
         AIChatResourceSnifferThrottleDelegate::InterceptedContent>();

components/ai_chat/renderer/page_content_extractor.cc

@@ -113,7 +113,7 @@ void PageContentExtractor::ExtractPageContent(
   if (intercepted_content_) {
     auto intercepted_content = std::move(intercepted_content_);
     intercepted_content_.reset();
-    VLOG(1) << "Using intercepted content.";
+    DVLOG(1) << "Using intercepted content.";
     DCHECK_EQ(intercepted_content->type,
               InterceptedContentType::kYouTubeMetadataString)
         << "Unexpected intercepted content type";
@@ -219,11 +219,7 @@ void PageContentExtractor::OnInterceptedPageContentChanged(
   // "page" change.
   mojo::AssociatedRemote<mojom::PageContentExtractorHost> host;
   render_frame()->GetRemoteAssociatedInterfaces()->GetInterface(&host);
-  if (host.is_bound()) {
-    host->OnInterceptedPageContentChanged();
-  } else {
-    DVLOG(1) << __func__ << " - no host bound to forward content event to.";
-  }
+  host->OnInterceptedPageContentChanged();
 }
 
 void PageContentExtractor::BindReceiver(

components/ai_chat/renderer/page_content_extractor.h

@@ -71,7 +71,7 @@ class PageContentExtractor
   int32_t isolated_world_id_;
 
   std::unique_ptr<AIChatResourceSnifferThrottleDelegate::InterceptedContent>
-      intercepted_content_ = nullptr;
+      intercepted_content_;
 
   base::WeakPtrFactory<PageContentExtractor> weak_ptr_factory_{this};
 };

components/ai_chat/renderer/yt_util.cc

@@ -16,7 +16,7 @@
 namespace ai_chat {
 
 std::optional<std::string> ChooseCaptionTrackUrl(
-    base::Value::List* caption_tracks) {
+    const base::Value::List* caption_tracks) {
   if (!caption_tracks || caption_tracks->empty()) {
     return std::nullopt;
   }
@@ -71,7 +71,8 @@ std::optional<std::string> ChooseCaptionTrackUrl(
   return *caption_url_raw;
 }
 
-std::optional<std::string> ParseAndChooseCaptionTrackUrl(std::string& body) {
+std::optional<std::string> ParseAndChooseCaptionTrackUrl(
+    std::string_view body) {
   if (!body.size()) {
     return std::nullopt;
   }
@@ -80,17 +81,17 @@ std::optional<std::string> ParseAndChooseCaptionTrackUrl(std::string& body) {
       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();
+    DVLOG(1) << __func__ << ": parsing error: " << result_value.ToString();
     return std::nullopt;
   } else if (!result_value->is_dict()) {
-    VLOG(1) << __func__ << ": parsing error: not a dict";
+    DVLOG(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";
+    DVLOG(1) << __func__ << ": no caption tracks found";
     return std::nullopt;
   }
 

components/ai_chat/renderer/yt_util.h

@@ -25,11 +25,11 @@ inline constexpr auto kYouTubeHosts =
 // Extract a caption url from an array of YT caption tracks, from the YT page
 // API.
 std::optional<std::string> ChooseCaptionTrackUrl(
-    base::Value::List* caption_tracks);
+    const base::Value::List* caption_tracks);
 
 // Parse YT metadata json string and choose the most appropriate caption track
 // url.
-std::optional<std::string> ParseAndChooseCaptionTrackUrl(std::string& body);
+std::optional<std::string> ParseAndChooseCaptionTrackUrl(std::string_view body);
 
 }  // namespace ai_chat
 

renderer/brave_url_loader_throttle_provider_impl.cc

@@ -101,7 +101,7 @@ BraveURLLoaderThrottleProviderImpl::CreateThrottles(
           ai_chat_resource_throttle =
               ai_chat::AIChatResourceSnifferThrottle::MaybeCreateThrottleFor(
                   page_content_delegate->GetWeakPtr(), request.url,
-                  base::SingleThreadTaskRunner::GetCurrentDefault());
+                  base::SequencedTaskRunner::GetCurrentDefault());
       if (ai_chat_resource_throttle) {
         throttles.emplace_back(std::move(ai_chat_resource_throttle));
       }