-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Add basics of ScalableIph keyed service. - This CL also implements a base class for writing browser_tests which interacts with ScalableIph service. Bug: b:284053005,b:284053020 Test: browser_tests --gtest_filter=*ScalableIphBrowserTest* Change-Id: Ief63ccee3be7449ea029c652c9805117823b0786 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4568407 Reviewed-by: David Trainor <dtrainor@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Commit-Queue: Yuki Awano <yawano@google.com> Reviewed-by: Alexander Alekseev <alemate@chromium.org> Reviewed-by: Tao Wu <wutao@chromium.org> Cr-Commit-Position: refs/heads/main@{#1152167}
- Loading branch information
Yuki Awano
authored and
Chromium LUCI CQ
committed
Jun 1, 2023
1 parent
ff4a42e
commit c695de2
Showing
16 changed files
with
463 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
# Copyright 2023 The Chromium Authors | ||
# Use of this source code is governed by a BSD-style license that can be | ||
# found in the LICENSE file. | ||
|
||
import("//build/config/chromeos/ui_mode.gni") | ||
|
||
assert(is_chromeos_ash) | ||
|
||
source_set("scalable_iph_factory") { | ||
sources = [ | ||
"scalable_iph_factory.cc", | ||
"scalable_iph_factory.h", | ||
] | ||
|
||
deps = [ | ||
"//base", | ||
"//chrome/browser", | ||
"//chrome/browser/profiles:profile", | ||
"//chromeos/ash/components/scalable_iph", | ||
"//components/feature_engagement/public", | ||
"//components/keyed_service/core", | ||
"//content/public/browser", | ||
] | ||
} | ||
|
||
source_set("browser_test_base") { | ||
testonly = true | ||
|
||
sources = [ | ||
"scalable_iph_browser_test_base.cc", | ||
"scalable_iph_browser_test_base.h", | ||
] | ||
|
||
deps = [ | ||
":scalable_iph_factory", | ||
"//base", | ||
"//chrome/browser", | ||
"//chrome/browser/profiles:profile", | ||
"//chrome/browser/ui", | ||
"//chrome/test:test_support_ui", | ||
"//components/feature_engagement/public", | ||
"//components/feature_engagement/test:test_support", | ||
"//components/keyed_service/content", | ||
"//components/keyed_service/core", | ||
"//content/public/browser", | ||
"//testing/gmock", | ||
] | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
mixins: "//chromeos/ash/components/scalable_iph/COMMON_METADATA" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
file://chromeos/ash/components/scalable_iph/OWNERS |
89 changes: 89 additions & 0 deletions
89
chrome/browser/ash/scalable_iph/scalable_iph_browser_test_base.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
// Copyright 2023 The Chromium Authors | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
#include "chrome/browser/ash/scalable_iph/scalable_iph_browser_test_base.h" | ||
|
||
#include <memory> | ||
|
||
#include "base/functional/bind.h" | ||
#include "chrome/browser/feature_engagement/tracker_factory.h" | ||
#include "chrome/browser/profiles/profile.h" | ||
#include "chrome/browser/ui/browser.h" | ||
#include "chrome/test/base/in_process_browser_test.h" | ||
#include "components/keyed_service/content/browser_context_dependency_manager.h" | ||
#include "components/keyed_service/core/keyed_service.h" | ||
#include "content/public/browser/browser_context.h" | ||
#include "testing/gmock/include/gmock/gmock.h" | ||
|
||
namespace ash { | ||
|
||
void ScalableIphBrowserTestBase::SetUp() { | ||
// Keyed service is a service which is tied to an object. For our use cases, | ||
// the object is `BrowserContext` (e.g. `Profile`). See | ||
// //components/keyed_service/README.md for details on keyed service. | ||
// | ||
// We set a testing factory to inject a mock. A testing factory must be set | ||
// early enough as a service is not created before that, e.g. a `Tracker` must | ||
// not be created before we set `CreateMockTracker`. If a keyed service is | ||
// created before we set our testing factory, `SetTestingFactory` will | ||
// destruct already created keyed services at a time we set our testing | ||
// factory. It destructs a keyed service at an unusual timing. It can trigger | ||
// a dangling pointer issue, etc. | ||
// | ||
// `SetUpOnMainThread` below is too late to set a testing factory. Note that | ||
// `InProcessBrowserTest::SetUp` is called at the very early stage, e.g. | ||
// before command lines are set, etc. | ||
subscription_ = | ||
BrowserContextDependencyManager::GetInstance() | ||
->RegisterCreateServicesCallbackForTesting( | ||
base::BindRepeating(&ScalableIphBrowserTestBase::CreateServices)); | ||
|
||
InProcessBrowserTest::SetUp(); | ||
} | ||
|
||
// `SetUpOnMainThread` is called just before a test body. Do the mock set up in | ||
// this function as `browser()` is not available in `SetUp` above. | ||
void ScalableIphBrowserTestBase::SetUpOnMainThread() { | ||
mock_tracker_ = static_cast<feature_engagement::test::MockTracker*>( | ||
feature_engagement::TrackerFactory::GetForBrowserContext( | ||
browser()->profile())); | ||
CHECK(mock_tracker_) | ||
<< "mock_tracker_ must be non-nullptr. GetForBrowserContext should " | ||
"create one via CreateMockTracker if it does not exist."; | ||
|
||
ON_CALL(*mock_tracker_, AddOnInitializedCallback) | ||
.WillByDefault( | ||
[](feature_engagement::Tracker::OnInitializedCallback callback) { | ||
std::move(callback).Run(true); | ||
}); | ||
|
||
ON_CALL(*mock_tracker_, IsInitialized).WillByDefault(testing::Return(true)); | ||
|
||
InProcessBrowserTest::SetUpOnMainThread(); | ||
} | ||
|
||
void ScalableIphBrowserTestBase::TearDownOnMainThread() { | ||
// We are going to release a reference to a MockTracker below. Verify the | ||
// expectation in advance to have a predictable behavior. | ||
testing::Mock::VerifyAndClearExpectations(mock_tracker_); | ||
mock_tracker_ = nullptr; | ||
|
||
InProcessBrowserTest::TearDownOnMainThread(); | ||
} | ||
|
||
// static | ||
void ScalableIphBrowserTestBase::CreateServices( | ||
content::BrowserContext* browser_context) { | ||
feature_engagement::TrackerFactory::GetInstance()->SetTestingFactory( | ||
browser_context, | ||
base::BindRepeating(&ScalableIphBrowserTestBase::CreateMockTracker)); | ||
} | ||
|
||
// static | ||
std::unique_ptr<KeyedService> ScalableIphBrowserTestBase::CreateMockTracker( | ||
content::BrowserContext* browser_context) { | ||
return std::make_unique<feature_engagement::test::MockTracker>(); | ||
} | ||
|
||
} // namespace ash |
42 changes: 42 additions & 0 deletions
42
chrome/browser/ash/scalable_iph/scalable_iph_browser_test_base.h
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
// Copyright 2023 The Chromium Authors | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
#ifndef CHROME_BROWSER_ASH_SCALABLE_IPH_SCALABLE_IPH_BROWSER_TEST_BASE_H_ | ||
#define CHROME_BROWSER_ASH_SCALABLE_IPH_SCALABLE_IPH_BROWSER_TEST_BASE_H_ | ||
|
||
#include <memory> | ||
|
||
#include "base/callback_list.h" | ||
#include "base/memory/raw_ptr.h" | ||
#include "chrome/test/base/in_process_browser_test.h" | ||
#include "components/feature_engagement/test/mock_tracker.h" | ||
#include "components/keyed_service/core/keyed_service.h" | ||
#include "content/public/browser/browser_context.h" | ||
|
||
namespace ash { | ||
|
||
class ScalableIphBrowserTestBase : public InProcessBrowserTest { | ||
public: | ||
// InProcessBrowserTest: | ||
void SetUp() override; | ||
void SetUpOnMainThread() override; | ||
void TearDownOnMainThread() override; | ||
|
||
protected: | ||
feature_engagement::test::MockTracker* mock_tracker() { | ||
return mock_tracker_; | ||
} | ||
|
||
private: | ||
static void CreateServices(content::BrowserContext* browser_context); | ||
static std::unique_ptr<KeyedService> CreateMockTracker( | ||
content::BrowserContext* browser_context); | ||
|
||
base::CallbackListSubscription subscription_; | ||
raw_ptr<feature_engagement::test::MockTracker> mock_tracker_; | ||
}; | ||
|
||
} // namespace ash | ||
|
||
#endif // CHROME_BROWSER_ASH_SCALABLE_IPH_SCALABLE_IPH_BROWSER_TEST_BASE_H_ |
28 changes: 28 additions & 0 deletions
28
chrome/browser/ash/scalable_iph/scalable_iph_browsertest.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
// Copyright 2023 The Chromium Authors | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
#include "chrome/browser/ash/scalable_iph/scalable_iph_browser_test_base.h" | ||
#include "chrome/browser/ash/scalable_iph/scalable_iph_factory.h" | ||
#include "chrome/browser/ui/browser.h" | ||
#include "chromeos/ash/components/scalable_iph/scalable_iph.h" | ||
#include "components/feature_engagement/test/mock_tracker.h" | ||
#include "content/public/test/browser_test.h" | ||
#include "testing/gmock/include/gmock/gmock.h" | ||
|
||
namespace { | ||
|
||
using ScalableIphBrowserTest = ash::ScalableIphBrowserTestBase; | ||
|
||
} // namespace | ||
|
||
IN_PROC_BROWSER_TEST_F(ScalableIphBrowserTest, RecordEvent) { | ||
EXPECT_CALL(*mock_tracker(), NotifyEvent("ScalableIphFiveMinTick")); | ||
|
||
scalable_iph::ScalableIph* scalable_iph = | ||
ash::ScalableIphFactory::GetForProfile(browser()->profile()); | ||
scalable_iph->RecordEvent(scalable_iph::ScalableIph::Event::kFiveMinTick); | ||
} | ||
|
||
// TODO(b/284053005): Add a test case for invalid event name. | ||
// TODO(b/284053005): Add a test case for checking trigger condition. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
// Copyright 2023 The Chromium Authors | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
#include "chrome/browser/ash/scalable_iph/scalable_iph_factory.h" | ||
|
||
#include "base/memory/singleton.h" | ||
#include "chrome/browser/feature_engagement/tracker_factory.h" | ||
#include "chrome/browser/profiles/profile_keyed_service_factory.h" | ||
#include "chromeos/ash/components/scalable_iph/scalable_iph.h" | ||
#include "components/feature_engagement/public/tracker.h" | ||
#include "components/keyed_service/core/keyed_service.h" | ||
#include "content/public/browser/browser_context.h" | ||
|
||
namespace ash { | ||
|
||
namespace { | ||
constexpr char kScalableIphServiceName[] = "ScalableIphKeyedService"; | ||
} | ||
|
||
ScalableIphFactory::ScalableIphFactory() | ||
: ProfileKeyedServiceFactory(kScalableIphServiceName) { | ||
DependsOn(feature_engagement::TrackerFactory::GetInstance()); | ||
} | ||
|
||
ScalableIphFactory::~ScalableIphFactory() = default; | ||
|
||
ScalableIphFactory* ScalableIphFactory::GetInstance() { | ||
return base::Singleton<ScalableIphFactory>::get(); | ||
} | ||
|
||
scalable_iph::ScalableIph* ScalableIphFactory::GetForProfile(Profile* profile) { | ||
return static_cast<scalable_iph::ScalableIph*>( | ||
GetInstance()->GetServiceForBrowserContext(profile, /*create=*/true)); | ||
} | ||
|
||
std::unique_ptr<KeyedService> | ||
ScalableIphFactory::BuildServiceInstanceForBrowserContext( | ||
content::BrowserContext* browser_context) const { | ||
feature_engagement::Tracker* tracker = | ||
feature_engagement::TrackerFactory::GetForBrowserContext(browser_context); | ||
CHECK(tracker); | ||
|
||
return std::make_unique<scalable_iph::ScalableIph>(tracker); | ||
} | ||
|
||
} // namespace ash |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
// Copyright 2023 The Chromium Authors | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
#ifndef CHROME_BROWSER_ASH_SCALABLE_IPH_SCALABLE_IPH_FACTORY_H_ | ||
#define CHROME_BROWSER_ASH_SCALABLE_IPH_SCALABLE_IPH_FACTORY_H_ | ||
|
||
#include "base/memory/singleton.h" | ||
#include "chrome/browser/profiles/profile.h" | ||
#include "chrome/browser/profiles/profile_keyed_service_factory.h" | ||
#include "chromeos/ash/components/scalable_iph/scalable_iph.h" | ||
#include "content/public/browser/browser_context.h" | ||
|
||
namespace ash { | ||
|
||
class ScalableIphFactory : public ProfileKeyedServiceFactory { | ||
public: | ||
static ScalableIphFactory* GetInstance(); | ||
static scalable_iph::ScalableIph* GetForProfile(Profile* profile); | ||
|
||
protected: | ||
// ProfileKeyedServiceFactory: | ||
std::unique_ptr<KeyedService> BuildServiceInstanceForBrowserContext( | ||
content::BrowserContext* browser_context) const override; | ||
|
||
private: | ||
friend struct base::DefaultSingletonTraits<ScalableIphFactory>; | ||
|
||
ScalableIphFactory(); | ||
~ScalableIphFactory() override; | ||
}; | ||
} // namespace ash | ||
|
||
#endif // CHROME_BROWSER_ASH_SCALABLE_IPH_SCALABLE_IPH_FACTORY_H_ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
# Copyright 2023 The Chromium Authors | ||
# Use of this source code is governed by a BSD-style license that can be | ||
# found in the LICENSE file. | ||
|
||
import("//build/config/chromeos/ui_mode.gni") | ||
|
||
assert(is_chromeos_ash) | ||
|
||
source_set("scalable_iph") { | ||
sources = [ | ||
"scalable_iph.cc", | ||
"scalable_iph.h", | ||
] | ||
|
||
deps = [ | ||
"//base", | ||
"//components/feature_engagement/public", | ||
"//components/keyed_service/core", | ||
] | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
os: CHROME_OS | ||
|
||
buganizer { | ||
component_id: 905229 | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
include_rules = [ | ||
"+components/feature_engagement/public", | ||
"+components/keyed_service/core", | ||
] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
mixins: "//chromeos/ash/components/scalable_iph/COMMON_METADATA" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
angelaxiao@chromium.org | ||
wutao@chromium.org | ||
xiaohuic@chromium.org | ||
yawano@google.com |
Oops, something went wrong.