Skip to content

Commit

Permalink
Refactor the PCA Model Manager and Service
Browse files Browse the repository at this point in the history 
Entirely removes the history annotations code path from the model
manager. Instead, the PCAService calls the BatchAnnotate APIs and
converts thew BatchAnnotationResults into the history annotations.

The existing batching in PCAService is repurposed to build the set of
inputs that is passed to the PCAModelManager.

Bug: 1278833
Change-Id: Ic79afcb7f923a7b83979325c3676312671a8aa28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3630871
Reviewed-by: Sophie Chang <sophiechang@chromium.org>
Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001883}
  • Loading branch information
Robert Ogden authored and Chromium LUCI CQ committed May 11, 2022
1 parent 4363d39 commit 4a1103b
Show file tree
Hide file tree
Showing 13 changed files with 316 additions and 1,096 deletions.
101 changes: 68 additions & 33 deletions chrome/browser/optimization_guide/page_content_annotations_service_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#include "components/history/core/browser/history_db_task.h"
#include "components/history/core/browser/history_service.h"
#include "components/optimization_guide/content/browser/page_content_annotations_service.h"
#include "components/optimization_guide/content/browser/test_page_content_annotator.h"
#include "components/optimization_guide/core/execution_status.h"
#include "components/optimization_guide/core/optimization_guide_enums.h"
#include "components/optimization_guide/core/optimization_guide_features.h"
#include "components/optimization_guide/core/optimization_guide_switches.h"
Expand Down Expand Up @@ -151,7 +153,7 @@ class PageContentAnnotationsServicePageTopicsBrowserTest
}
~PageContentAnnotationsServicePageTopicsBrowserTest() override = default;

void LoadModel() {
void LoadPageTopicsV2Model() {
proto::Any any_metadata;
any_metadata.set_type_url(
"type.googleapis.com/com.foo.PageTopicsModelMetadata");
Expand All @@ -176,8 +178,6 @@ class PageContentAnnotationsServicePageTopicsBrowserTest
.AppendASCII("optimization_guide")
.AppendASCII("page_topics_128_model.tflite");

base::HistogramTester histogram_tester;

OptimizationGuideKeyedServiceFactory::GetForProfile(browser()->profile())
->OverrideTargetModelForTesting(
proto::OPTIMIZATION_TARGET_PAGE_TOPICS_V2,
Expand All @@ -186,9 +186,21 @@ class PageContentAnnotationsServicePageTopicsBrowserTest
.SetModelMetadata(any_metadata)
.Build());

RetryForHistogramUntilCountReached(
&histogram_tester,
"OptimizationGuide.ModelExecutor.ModelFileUpdated.PageTopicsV2", 1);
PageContentAnnotationsService* service =
PageContentAnnotationsServiceFactory::GetForProfile(
browser()->profile());
ASSERT_TRUE(service);

base::RunLoop run_loop;
service->RequestAndNotifyWhenModelAvailable(
AnnotationType::kPageTopics,
base::BindOnce(
[](base::RunLoop* run_loop, bool success) {
EXPECT_TRUE(success);
run_loop->Quit();
},
&run_loop));
run_loop.Run();
}

private:
Expand All @@ -206,11 +218,7 @@ IN_PROC_BROWSER_TEST_F(PageContentAnnotationsServicePageTopicsBrowserTest,
PageContentAnnotationsService* service =
PageContentAnnotationsServiceFactory::GetForProfile(browser()->profile());
ASSERT_TRUE(service);

service->RequestAndNotifyWhenModelAvailable(AnnotationType::kPageTopics,
base::DoNothing());

LoadModel();
LoadPageTopicsV2Model();

std::vector<BatchAnnotationResult> results;
base::RunLoop run_loop;
Expand Down Expand Up @@ -328,7 +336,7 @@ class PageContentAnnotationsServiceBrowserTest : public InProcessBrowserTest {

OptimizationGuideKeyedServiceFactory::GetForProfile(browser()->profile())
->OverrideTargetModelForTesting(
proto::OPTIMIZATION_TARGET_PAGE_TOPICS,
proto::OPTIMIZATION_TARGET_PAGE_VISIBILITY,
optimization_guide::TestModelInfoBuilder()
.SetModelFilePath(model_file_path)
.SetModelMetadata(any_metadata)
Expand All @@ -337,7 +345,7 @@ class PageContentAnnotationsServiceBrowserTest : public InProcessBrowserTest {
#if BUILDFLAG(BUILD_WITH_TFLITE_LIB)
RetryForHistogramUntilCountReached(
&histogram_tester,
"OptimizationGuide.ModelExecutor.ModelFileUpdated.PageTopics", 1);
"OptimizationGuide.ModelExecutor.ModelFileUpdated.PageVisibility", 1);
#else
base::RunLoop().RunUntilIdle();
#endif
Expand Down Expand Up @@ -411,6 +419,10 @@ IN_PROC_BROWSER_TEST_F(PageContentAnnotationsServiceBrowserTest,
histogram_tester.ExpectUniqueSample(
"OptimizationGuide.PageContentAnnotationsService.ContentAnnotated", true,
1);
#else
histogram_tester.ExpectUniqueSample(
"OptimizationGuide.PageContentAnnotationsService.ContentAnnotated", false,
1);
#endif

#if BUILDFLAG(BUILD_WITH_TFLITE_LIB)
Expand All @@ -434,10 +446,7 @@ IN_PROC_BROWSER_TEST_F(PageContentAnnotationsServiceBrowserTest,
GetContentAnnotationsForURL(url);
ASSERT_TRUE(got_content_annotations.has_value());
EXPECT_NE(-1.0, got_content_annotations->model_annotations.visibility_score);
EXPECT_FALSE(got_content_annotations->model_annotations.categories.empty());
EXPECT_EQ(
123,
got_content_annotations->model_annotations.page_topics_model_version);
EXPECT_TRUE(got_content_annotations->model_annotations.categories.empty());

auto entries = ukm_recorder.GetEntriesByName(
ukm::builders::PageContentAnnotations::kEntryName);
Expand Down Expand Up @@ -677,8 +686,41 @@ class PageContentAnnotationsServiceBatchVisitTest
}
~PageContentAnnotationsServiceBatchVisitTest() override = default;

void SetUpOnMainThread() override {
PageContentAnnotationsServiceNoHistoryTest::SetUpOnMainThread();

PageContentAnnotationsService* service =
PageContentAnnotationsServiceFactory::GetForProfile(
browser()->profile());

annotator_.UsePageEntities(
/*model_info=*/absl::nullopt,
{
{
"Test Page",
{
ScoredEntityMetadata(0.6,
EntityMetadata("test", "test", {})),
ScoredEntityMetadata(0.4,
EntityMetadata("page", "page", {})),
},
},
{
"sometext",
{
ScoredEntityMetadata(0.7,
EntityMetadata("some", "some", {})),
ScoredEntityMetadata(0.3,
EntityMetadata("text", "text", {})),
},
},
});
service->OverridePageContentAnnotatorForTesting(&annotator_);
}

private:
base::test::ScopedFeatureList scoped_feature_list_;
TestPageContentAnnotator annotator_;
};

IN_PROC_BROWSER_TEST_F(PageContentAnnotationsServiceBatchVisitTest,
Expand Down Expand Up @@ -735,13 +777,6 @@ class PageContentAnnotationsServiceBatchVisitNoAnnotateTest
}
~PageContentAnnotationsServiceBatchVisitNoAnnotateTest() override = default;

void SetUpCommandLine(base::CommandLine* cmd) override {
// Note: the code after the early return this disables is well tested in
// other places.
cmd->AppendSwitch(
optimization_guide::switches::kStopHistoryVisitBatchAnnotateForTesting);
}

private:
base::test::ScopedFeatureList scoped_feature_list_;
};
Expand Down Expand Up @@ -846,11 +881,11 @@ IN_PROC_BROWSER_TEST_F(PageContentAnnotationsServiceModelNotLoadedOnStartupTest,

RetryForHistogramUntilCountReached(
&histogram_tester,
"OptimizationGuide.PageContentAnnotationsService.ModelAvailable", 1);
"OptimizationGuide.ModelExecutor.ExecutionStatus.PageVisibility", 1);

histogram_tester.ExpectUniqueSample(
"OptimizationGuide.PageContentAnnotationsService.ModelAvailable", false,
1);
"OptimizationGuide.ModelExecutor.ExecutionStatus.PageVisibility",
ExecutionStatus::kErrorModelFileNotAvailable, 1);

LoadAndWaitForModel();

Expand All @@ -860,16 +895,16 @@ IN_PROC_BROWSER_TEST_F(PageContentAnnotationsServiceModelNotLoadedOnStartupTest,

RetryForHistogramUntilCountReached(
&histogram_tester,
"OptimizationGuide.PageContentAnnotationsService.ModelAvailable", 2);
"OptimizationGuide.ModelExecutor.ExecutionStatus.PageVisibility", 2);

histogram_tester.ExpectBucketCount(
"OptimizationGuide.PageContentAnnotationsService.ModelAvailable", false,
1);
"OptimizationGuide.ModelExecutor.ExecutionStatus.PageVisibility",
ExecutionStatus::kErrorModelFileNotAvailable, 1);
histogram_tester.ExpectBucketCount(
"OptimizationGuide.PageContentAnnotationsService.ModelAvailable", true,
1);
"OptimizationGuide.ModelExecutor.ExecutionStatus.PageVisibility",
ExecutionStatus::kSuccess, 1);
histogram_tester.ExpectTotalCount(
"OptimizationGuide.PageContentAnnotationsService.ModelAvailable", 2);
"OptimizationGuide.ModelExecutor.ExecutionStatus.PageVisibility", 2);
}

class PageContentAnnotationsServiceValidationTest
Expand Down

0 comments on commit 4a1103b

Please sign in to comment.