Commit 76b5e082 authored by Tarun Bansal's avatar Tarun Bansal Committed by Commit Bot

NQE servicification of Chrome Interventions Internals

Also, verified using local Chrome android build. Network
quality change messages appeared on chrome://interventions-internals

Change-Id: I9b958b7fff922fdf8af369c5ba6e29872ceac6dc
Bug: 819244
Reviewed-on: https://chromium-review.googlesource.com/1175554Reviewed-by: default avatarRyan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583357}
parent 3431c515
...@@ -12,12 +12,13 @@ ...@@ -12,12 +12,13 @@
#include "base/metrics/field_trial_params.h" #include "base/metrics/field_trial_params.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/flag_descriptions.h" #include "chrome/browser/flag_descriptions.h"
#include "chrome/browser/net/nqe/ui_network_quality_estimator_service.h"
#include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_switches.h"
#include "components/previews/core/previews_experiments.h" #include "components/previews/core/previews_experiments.h"
#include "components/previews/core/previews_switches.h" #include "components/previews/core/previews_switches.h"
#include "net/nqe/network_quality_estimator_params.h" #include "net/nqe/network_quality_estimator_params.h"
#include "services/network/public/cpp/network_quality_tracker.h"
#include "services/network/public/cpp/network_switches.h" #include "services/network/public/cpp/network_switches.h"
namespace { namespace {
...@@ -99,11 +100,9 @@ std::string GetEnabledStateForSwitch(const std::string& switch_name) { ...@@ -99,11 +100,9 @@ std::string GetEnabledStateForSwitch(const std::string& switch_name) {
InterventionsInternalsPageHandler::InterventionsInternalsPageHandler( InterventionsInternalsPageHandler::InterventionsInternalsPageHandler(
mojom::InterventionsInternalsPageHandlerRequest request, mojom::InterventionsInternalsPageHandlerRequest request,
previews::PreviewsUIService* previews_ui_service, previews::PreviewsUIService* previews_ui_service)
UINetworkQualityEstimatorService* ui_nqe_service)
: binding_(this, std::move(request)), : binding_(this, std::move(request)),
previews_ui_service_(previews_ui_service), previews_ui_service_(previews_ui_service),
ui_nqe_service_(ui_nqe_service),
current_estimated_ect_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN) { current_estimated_ect_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN) {
logger_ = previews_ui_service_->previews_logger(); logger_ = previews_ui_service_->previews_logger();
DCHECK(logger_); DCHECK(logger_);
...@@ -112,7 +111,8 @@ InterventionsInternalsPageHandler::InterventionsInternalsPageHandler( ...@@ -112,7 +111,8 @@ InterventionsInternalsPageHandler::InterventionsInternalsPageHandler(
InterventionsInternalsPageHandler::~InterventionsInternalsPageHandler() { InterventionsInternalsPageHandler::~InterventionsInternalsPageHandler() {
DCHECK(logger_); DCHECK(logger_);
logger_->RemoveObserver(this); logger_->RemoveObserver(this);
ui_nqe_service_->RemoveEffectiveConnectionTypeObserver(this); g_browser_process->network_quality_tracker()
->RemoveEffectiveConnectionTypeObserver(this);
} }
void InterventionsInternalsPageHandler::SetClientPage( void InterventionsInternalsPageHandler::SetClientPage(
...@@ -120,7 +120,8 @@ void InterventionsInternalsPageHandler::SetClientPage( ...@@ -120,7 +120,8 @@ void InterventionsInternalsPageHandler::SetClientPage(
page_ = std::move(page); page_ = std::move(page);
DCHECK(page_); DCHECK(page_);
logger_->AddAndNotifyObserver(this); logger_->AddAndNotifyObserver(this);
ui_nqe_service_->AddEffectiveConnectionTypeObserver(this); g_browser_process->network_quality_tracker()
->AddEffectiveConnectionTypeObserver(this);
} }
void InterventionsInternalsPageHandler::OnEffectiveConnectionTypeChanged( void InterventionsInternalsPageHandler::OnEffectiveConnectionTypeChanged(
......
...@@ -15,19 +15,16 @@ ...@@ -15,19 +15,16 @@
#include "components/previews/core/previews_logger_observer.h" #include "components/previews/core/previews_logger_observer.h"
#include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/bindings/binding.h"
#include "net/nqe/effective_connection_type.h" #include "net/nqe/effective_connection_type.h"
#include "net/nqe/effective_connection_type_observer.h" #include "services/network/public/cpp/network_quality_tracker.h"
class UINetworkQualityEstimatorService;
class InterventionsInternalsPageHandler class InterventionsInternalsPageHandler
: public previews::PreviewsLoggerObserver, : public previews::PreviewsLoggerObserver,
public net::EffectiveConnectionTypeObserver, public network::NetworkQualityTracker::EffectiveConnectionTypeObserver,
public mojom::InterventionsInternalsPageHandler { public mojom::InterventionsInternalsPageHandler {
public: public:
InterventionsInternalsPageHandler( InterventionsInternalsPageHandler(
mojom::InterventionsInternalsPageHandlerRequest request, mojom::InterventionsInternalsPageHandlerRequest request,
previews::PreviewsUIService* previews_ui_service, previews::PreviewsUIService* previews_ui_service);
UINetworkQualityEstimatorService* ui_nqe_service);
~InterventionsInternalsPageHandler() override; ~InterventionsInternalsPageHandler() override;
// mojom::InterventionsInternalsPageHandler: // mojom::InterventionsInternalsPageHandler:
...@@ -47,7 +44,7 @@ class InterventionsInternalsPageHandler ...@@ -47,7 +44,7 @@ class InterventionsInternalsPageHandler
void OnLastObserverRemove() override; void OnLastObserverRemove() override;
private: private:
// net::EffectiveConnectionTypeObserver: // network::NetworkQualityTracker::EffectiveConnectionTypeObserver:
void OnEffectiveConnectionTypeChanged( void OnEffectiveConnectionTypeChanged(
net::EffectiveConnectionType type) override; net::EffectiveConnectionType type) override;
...@@ -61,10 +58,6 @@ class InterventionsInternalsPageHandler ...@@ -61,10 +58,6 @@ class InterventionsInternalsPageHandler
// guaranteed to outlive |this|. // guaranteed to outlive |this|.
previews::PreviewsUIService* previews_ui_service_; previews::PreviewsUIService* previews_ui_service_;
// A pointer to the UINetworkQualityEsitmatorService, guaranteed to outlive
// |this|.
UINetworkQualityEstimatorService* ui_nqe_service_;
// The current estimated effective connection type. // The current estimated effective connection type.
net::EffectiveConnectionType current_estimated_ect_; net::EffectiveConnectionType current_estimated_ect_;
......
...@@ -22,7 +22,6 @@ ...@@ -22,7 +22,6 @@
#include "base/time/default_clock.h" #include "base/time/default_clock.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/flag_descriptions.h" #include "chrome/browser/flag_descriptions.h"
#include "chrome/browser/net/nqe/ui_network_quality_estimator_service.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/webui/interventions_internals/interventions_internals.mojom.h" #include "chrome/browser/ui/webui/interventions_internals/interventions_internals.mojom.h"
#include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_constants.h"
...@@ -191,27 +190,6 @@ class TestPreviewsLogger : public previews::PreviewsLogger { ...@@ -191,27 +190,6 @@ class TestPreviewsLogger : public previews::PreviewsLogger {
bool remove_is_called_; bool remove_is_called_;
}; };
// Mock class to test interaction between PageHandler and the
// UINetworkQualityEstimatorService.
class TestUINetworkQualityEstimatorService
: public UINetworkQualityEstimatorService {
public:
explicit TestUINetworkQualityEstimatorService(Profile* profile)
: UINetworkQualityEstimatorService(profile), remove_is_called_(false) {}
// UINetworkQualityEstimatorService:
void RemoveEffectiveConnectionTypeObserver(
net::EffectiveConnectionTypeObserver* observer) override {
remove_is_called_ = true;
}
bool RemovedObserverIsCalled() const { return remove_is_called_; }
private:
// Check if the observer removed itself from the observer list.
bool remove_is_called_;
};
// A dummy class to setup PreviewsUIService. // A dummy class to setup PreviewsUIService.
class TestPreviewsDeciderImpl : public previews::PreviewsDeciderImpl { class TestPreviewsDeciderImpl : public previews::PreviewsDeciderImpl {
public: public:
...@@ -276,16 +254,11 @@ class InterventionsInternalsPageHandlerTest : public testing::Test { ...@@ -276,16 +254,11 @@ class InterventionsInternalsPageHandlerTest : public testing::Test {
std::make_unique<TestPreviewsUIService>(&io_data, std::move(logger)); std::make_unique<TestPreviewsUIService>(&io_data, std::move(logger));
ASSERT_TRUE(profile_manager_.SetUp()); ASSERT_TRUE(profile_manager_.SetUp());
TestingProfile* test_profile =
profile_manager_.CreateTestingProfile(chrome::kInitialProfile);
ui_nqe_service_ =
std::make_unique<TestUINetworkQualityEstimatorService>(test_profile);
mojom::InterventionsInternalsPageHandlerPtr page_handler_ptr; mojom::InterventionsInternalsPageHandlerPtr page_handler_ptr;
handler_request_ = mojo::MakeRequest(&page_handler_ptr); handler_request_ = mojo::MakeRequest(&page_handler_ptr);
page_handler_ = std::make_unique<InterventionsInternalsPageHandler>( page_handler_ = std::make_unique<InterventionsInternalsPageHandler>(
std::move(handler_request_), previews_ui_service_.get(), std::move(handler_request_), previews_ui_service_.get());
ui_nqe_service_.get());
mojom::InterventionsInternalsPagePtr page_ptr; mojom::InterventionsInternalsPagePtr page_ptr;
page_request_ = mojo::MakeRequest(&page_ptr); page_request_ = mojo::MakeRequest(&page_ptr);
...@@ -306,7 +279,6 @@ class InterventionsInternalsPageHandlerTest : public testing::Test { ...@@ -306,7 +279,6 @@ class InterventionsInternalsPageHandlerTest : public testing::Test {
TestPreviewsLogger* logger_; TestPreviewsLogger* logger_;
std::unique_ptr<TestPreviewsUIService> previews_ui_service_; std::unique_ptr<TestPreviewsUIService> previews_ui_service_;
std::unique_ptr<TestUINetworkQualityEstimatorService> ui_nqe_service_;
// InterventionsInternalPageHandler's variables. // InterventionsInternalPageHandler's variables.
mojom::InterventionsInternalsPageHandlerRequest handler_request_; mojom::InterventionsInternalsPageHandlerRequest handler_request_;
...@@ -650,6 +622,13 @@ TEST_F(InterventionsInternalsPageHandlerTest, OnNewMessageLogAddedPostToPage) { ...@@ -650,6 +622,13 @@ TEST_F(InterventionsInternalsPageHandlerTest, OnNewMessageLogAddedPostToPage) {
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
mojom::MessageLogPtr* actual = page_->message(); mojom::MessageLogPtr* actual = page_->message();
// Discard any messages generated by network quality tracker.
while ((*actual)->type == "ECT Changed") {
page_handler_->OnNewMessageLogAdded(message);
base::RunLoop().RunUntilIdle();
actual = page_->message();
}
EXPECT_EQ(message.event_type, (*actual)->type); EXPECT_EQ(message.event_type, (*actual)->type);
EXPECT_EQ(message.event_description, (*actual)->description); EXPECT_EQ(message.event_description, (*actual)->description);
EXPECT_EQ(message.url, (*actual)->url); EXPECT_EQ(message.url, (*actual)->url);
...@@ -661,10 +640,8 @@ TEST_F(InterventionsInternalsPageHandlerTest, OnNewMessageLogAddedPostToPage) { ...@@ -661,10 +640,8 @@ TEST_F(InterventionsInternalsPageHandlerTest, OnNewMessageLogAddedPostToPage) {
TEST_F(InterventionsInternalsPageHandlerTest, ObserverIsRemovedWhenDestroyed) { TEST_F(InterventionsInternalsPageHandlerTest, ObserverIsRemovedWhenDestroyed) {
EXPECT_FALSE(logger_->RemovedObserverIsCalled()); EXPECT_FALSE(logger_->RemovedObserverIsCalled());
EXPECT_FALSE(ui_nqe_service_->RemovedObserverIsCalled());
page_handler_.reset(); page_handler_.reset();
EXPECT_TRUE(logger_->RemovedObserverIsCalled()); EXPECT_TRUE(logger_->RemovedObserverIsCalled());
EXPECT_TRUE(ui_nqe_service_->RemovedObserverIsCalled());
} }
TEST_F(InterventionsInternalsPageHandlerTest, OnNewBlacklistedHostPostToPage) { TEST_F(InterventionsInternalsPageHandlerTest, OnNewBlacklistedHostPostToPage) {
......
...@@ -8,8 +8,6 @@ ...@@ -8,8 +8,6 @@
#include <utility> #include <utility>
#include <vector> #include <vector>
#include "chrome/browser/net/nqe/ui_network_quality_estimator_service.h"
#include "chrome/browser/net/nqe/ui_network_quality_estimator_service_factory.h"
#include "chrome/browser/previews/previews_service.h" #include "chrome/browser/previews/previews_service.h"
#include "chrome/browser/previews/previews_service_factory.h" #include "chrome/browser/previews/previews_service_factory.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
...@@ -58,8 +56,6 @@ InterventionsInternalsUI::InterventionsInternalsUI(content::WebUI* web_ui) ...@@ -58,8 +56,6 @@ InterventionsInternalsUI::InterventionsInternalsUI(content::WebUI* web_ui)
} }
content::WebUIDataSource::Add(profile, GetSource()); content::WebUIDataSource::Add(profile, GetSource());
previews_ui_service_ = previews_service->previews_ui_service(); previews_ui_service_ = previews_service->previews_ui_service();
ui_nqe_service_ =
UINetworkQualityEstimatorServiceFactory::GetForProfile(profile);
AddHandlerToRegistry(base::BindRepeating( AddHandlerToRegistry(base::BindRepeating(
&InterventionsInternalsUI::BindInterventionsInternalsPageHandler, &InterventionsInternalsUI::BindInterventionsInternalsPageHandler,
base::Unretained(this))); base::Unretained(this)));
...@@ -70,7 +66,6 @@ InterventionsInternalsUI::~InterventionsInternalsUI() {} ...@@ -70,7 +66,6 @@ InterventionsInternalsUI::~InterventionsInternalsUI() {}
void InterventionsInternalsUI::BindInterventionsInternalsPageHandler( void InterventionsInternalsUI::BindInterventionsInternalsPageHandler(
mojom::InterventionsInternalsPageHandlerRequest request) { mojom::InterventionsInternalsPageHandlerRequest request) {
DCHECK(previews_ui_service_); DCHECK(previews_ui_service_);
DCHECK(ui_nqe_service_);
page_handler_.reset(new InterventionsInternalsPageHandler( page_handler_.reset(new InterventionsInternalsPageHandler(
std::move(request), previews_ui_service_, ui_nqe_service_)); std::move(request), previews_ui_service_));
} }
...@@ -14,8 +14,6 @@ namespace previews { ...@@ -14,8 +14,6 @@ namespace previews {
class PreviewsUIService; class PreviewsUIService;
} // namespace previews } // namespace previews
class UINetworkQualityEstimatorService;
// The WebUI for chrome://interventions-internals. // The WebUI for chrome://interventions-internals.
class InterventionsInternalsUI : public ui::MojoWebUIController { class InterventionsInternalsUI : public ui::MojoWebUIController {
public: public:
...@@ -29,10 +27,6 @@ class InterventionsInternalsUI : public ui::MojoWebUIController { ...@@ -29,10 +27,6 @@ class InterventionsInternalsUI : public ui::MojoWebUIController {
// The PreviewsUIService associated with this UI. // The PreviewsUIService associated with this UI.
previews::PreviewsUIService* previews_ui_service_; previews::PreviewsUIService* previews_ui_service_;
// The network quality estimator service for getting the estimate effective
// conntection type.
UINetworkQualityEstimatorService* ui_nqe_service_;
std::unique_ptr<InterventionsInternalsPageHandler> page_handler_; std::unique_ptr<InterventionsInternalsPageHandler> page_handler_;
DISALLOW_COPY_AND_ASSIGN(InterventionsInternalsUI); DISALLOW_COPY_AND_ASSIGN(InterventionsInternalsUI);
......
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