Commit c58f0f1a authored by Donn Denman's avatar Donn Denman Committed by Commit Bot

[TTS] Fix crash with Ranker holding WebContents.

The native ContextualSearchRankerLoggerImpl was holding on to a
WebContents pointer, which is somewhat dangerous. We're seeing
crashes due to the WebContents going stale.

This change just uses the WebContents to get the UKM SourceId
and hangs on to it instead.  The SourceId is all that's needed
when we write the log.  Now we also invalidate the SourceId
when the log is written to ensure that we don't try to write
more than once per SetupLoggingAndRanker call.

BUG=795936

Change-Id: I4d1363ae709263bc76fc54f4c6316963c12f5018
Reviewed-on: https://chromium-review.googlesource.com/836127Reviewed-by: default avatarTheresa <twellington@chromium.org>
Reviewed-by: default avatarRoger McFarlane <rogerm@chromium.org>
Commit-Queue: Donn Denman <donnd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525339}
parent 9d022abd
......@@ -41,22 +41,25 @@ void ContextualSearchRankerLoggerImpl::SetupLoggingAndRanker(
JNIEnv* env,
jobject obj,
const base::android::JavaParamRef<jobject>& java_web_contents) {
web_contents_ = content::WebContents::FromJavaWebContents(java_web_contents);
if (!web_contents_)
content::WebContents* web_contents =
content::WebContents::FromJavaWebContents(java_web_contents);
if (!web_contents)
return;
SetupRankerPredictor();
source_id_ = ukm::GetSourceIdForWebContentsDocument(web_contents);
SetupRankerPredictor(*web_contents);
// Start building example data based on features to be gathered and logged.
ranker_example_ = std::make_unique<assist_ranker::RankerExample>();
}
void ContextualSearchRankerLoggerImpl::SetupRankerPredictor() {
void ContextualSearchRankerLoggerImpl::SetupRankerPredictor(
const content::WebContents& web_contents) {
// Create one predictor for the current BrowserContext.
if (browser_context_) {
DCHECK(browser_context_ == web_contents_->GetBrowserContext());
DCHECK(browser_context_ == web_contents.GetBrowserContext());
return;
}
browser_context_ = web_contents_->GetBrowserContext();
browser_context_ = web_contents.GetBrowserContext();
assist_ranker::AssistRankerService* assist_ranker_service =
assist_ranker::AssistRankerServiceFactory::GetForBrowserContext(
......@@ -119,10 +122,9 @@ AssistRankerPrediction ContextualSearchRankerLoggerImpl::RunInference(
void ContextualSearchRankerLoggerImpl::WriteLogAndReset(JNIEnv* env,
jobject obj) {
if (predictor_ && ranker_example_) {
ukm::SourceId source_id =
ukm::GetSourceIdForWebContentsDocument(web_contents_);
predictor_->LogExampleToUkm(*ranker_example_.get(), source_id);
if (predictor_ && ranker_example_ && source_id_ != ukm::kInvalidSourceId) {
predictor_->LogExampleToUkm(*ranker_example_.get(), source_id_);
source_id_ = ukm::kInvalidSourceId;
}
has_predicted_decision_ = false;
ranker_example_ = std::make_unique<assist_ranker::RankerExample>();
......
......@@ -7,6 +7,7 @@
#include "base/android/jni_android.h"
#include "base/memory/weak_ptr.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
namespace content {
class BrowserContext;
......@@ -73,12 +74,10 @@ class ContextualSearchRankerLoggerImpl {
void LogFeature(const std::string& feature_name, int value);
// Sets up the Ranker Predictor for the given |web_contents|.
void SetupRankerPredictor();
void SetupRankerPredictor(const content::WebContents& web_contents);
// The WebContents object used to produce the source_id for UKMs, and to get
// browser_context when fetching the predictor. The object is not owned by
// ContextualSearchRankerLoggerImpl.
content::WebContents* web_contents_ = nullptr;
// The source_id for UKMs for the current page.
ukm::SourceId source_id_ = ukm::kInvalidSourceId;
// The Ranker Predictor for whether a tap gesture should be suppressed or not.
base::WeakPtr<assist_ranker::BinaryClassifierPredictor> predictor_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment