Skip to content

Commit

Permalink
Add network traffic annotation for GCDApiFlow.
Browse files Browse the repository at this point in the history
Clean up some cruft in the process.

BUG=656607

Review-Url: https://codereview.chromium.org/2792303002
Cr-Commit-Position: refs/heads/master@{#462213}
  • Loading branch information
leizleiz authored and Commit bot committed Apr 5, 2017
1 parent 0aaeb2d commit 413a545
Show file tree
Hide file tree
Showing 14 changed files with 115 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@

#include "chrome/browser/printing/cloud_print/cloud_print_printer_list.h"

#include "base/values.h"
#include "chrome/common/cloud_print/cloud_print_constants.h"
#include "components/cloud_devices/common/cloud_devices_urls.h"

namespace cloud_print {

CloudPrintPrinterList::Delegate::Delegate() {}

CloudPrintPrinterList::Delegate::~Delegate() {}

CloudPrintPrinterList::CloudPrintPrinterList(Delegate* delegate)
Expand Down Expand Up @@ -51,6 +50,11 @@ GURL CloudPrintPrinterList::GetURL() {
return cloud_devices::GetCloudPrintRelativeURL("search");
}

GCDApiFlow::Request::NetworkTrafficAnnotation
CloudPrintPrinterList::GetNetworkTrafficAnnotationType() {
return TYPE_SEARCH;
}

bool CloudPrintPrinterList::FillPrinterDetails(
const base::DictionaryValue& printer_value,
Device* printer_details) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <string>
#include <vector>

#include "base/values.h"
#include "chrome/browser/printing/cloud_print/gcd_api_flow.h"

namespace cloud_print {
Expand All @@ -24,7 +23,6 @@ class CloudPrintPrinterList : public CloudPrintApiFlowRequest {

class Delegate {
public:
Delegate();
virtual ~Delegate();

virtual void OnDeviceListReady(const DeviceList& devices) = 0;
Expand All @@ -38,6 +36,7 @@ class CloudPrintPrinterList : public CloudPrintApiFlowRequest {
void OnGCDApiFlowError(GCDApiFlow::Status status) override;
void OnGCDApiFlowComplete(const base::DictionaryValue& value) override;
GURL GetURL() override;
NetworkTrafficAnnotation GetNetworkTrafficAnnotationType() override;

private:
bool FillPrinterDetails(const base::DictionaryValue& printer_value,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <set>

#include "base/json/json_reader.h"
#include "base/values.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"

Expand Down Expand Up @@ -44,7 +45,6 @@ TEST(CloudPrintPrinterListTest, Params) {
device_list.GetURL());
EXPECT_EQ("https://www.googleapis.com/auth/cloudprint",
device_list.GetOAuthScope());
EXPECT_EQ(net::URLFetcher::GET, device_list.GetRequestType());
EXPECT_FALSE(device_list.GetExtraRequestHeaders().empty());
}

Expand All @@ -62,16 +62,14 @@ TEST(CloudPrintPrinterListTest, Parsing) {

Mock::VerifyAndClear(&delegate);

std::set<std::string> ids_found;
std::set<std::string> ids_expected;
ids_expected.insert("someID");

for (size_t i = 0; i != devices.size(); ++i) {
ids_found.insert(devices[i].id);
}
std::set<std::string> ids_found;
for (const auto& device : devices)
ids_found.insert(device.id);

ASSERT_EQ(ids_expected, ids_found);

EXPECT_EQ("someID", devices[0].id);
EXPECT_EQ("someDisplayName", devices[0].display_name);
EXPECT_EQ("someDescription", devices[0].description);
Expand Down
7 changes: 0 additions & 7 deletions chrome/browser/printing/cloud_print/gcd_api_flow.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,9 @@

namespace cloud_print {

GCDApiFlow::Request::Request() {
}

GCDApiFlow::Request::~Request() {
}

net::URLFetcher::RequestType GCDApiFlow::Request::GetRequestType() {
return net::URLFetcher::GET;
}

std::unique_ptr<GCDApiFlow> GCDApiFlow::Create(
net::URLRequestContextGetter* request_context,
OAuth2TokenService* token_service,
Expand Down
22 changes: 17 additions & 5 deletions chrome/browser/printing/cloud_print/gcd_api_flow.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@

#include "base/macros.h"
#include "google_apis/gaia/oauth2_token_service.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
#include "net/url_request/url_fetcher.h"
#include "net/url_request/url_request_context_getter.h"

namespace base {
class DictionaryValue;
}

namespace cloud_print {

// API flow for communicating with cloud print and cloud devices.
Expand All @@ -32,23 +37,30 @@ class GCDApiFlow {
// Parses results of requests.
class Request {
public:
Request();
enum NetworkTrafficAnnotation {
TYPE_SEARCH,
TYPE_PRIVET_REGISTER,
};

virtual ~Request();

// Called if the API flow fails.
virtual void OnGCDApiFlowError(Status status) = 0;

// Called when the API flow finishes.
virtual void OnGCDApiFlowComplete(const base::DictionaryValue& value) = 0;

// Returns the URL for this request.
virtual GURL GetURL() = 0;

// Returns the scope parameter for use with OAuth.
virtual std::string GetOAuthScope() = 0;

virtual net::URLFetcher::RequestType GetRequestType();

// Returns extra headers, if any, to send with this request.
virtual std::vector<std::string> GetExtraRequestHeaders() = 0;

private:
DISALLOW_COPY_AND_ASSIGN(Request);
// Returns the network traffic annotation tag for this request.
virtual NetworkTrafficAnnotation GetNetworkTrafficAnnotationType() = 0;
};

GCDApiFlow();
Expand Down
67 changes: 57 additions & 10 deletions chrome/browser/printing/cloud_print/gcd_api_flow_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,58 @@
#include "net/http/http_status_code.h"
#include "net/url_request/url_request_status.h"

using net::DefineNetworkTrafficAnnotation;

namespace cloud_print {

namespace {

const char kCloudPrintOAuthHeaderFormat[] = "Authorization: Bearer %s";

net::NetworkTrafficAnnotationTag GetNetworkTrafficAnnotation(
GCDApiFlow::Request::NetworkTrafficAnnotation type) {
if (type == CloudPrintApiFlowRequest::TYPE_PRIVET_REGISTER) {
return DefineNetworkTrafficAnnotation("cloud_print_privet_register", R"(
semantics {
sender: "Cloud Print"
description:
"Registers a locally discovered Privet printer with a Cloud Print "
"Server.
trigger:
"Users can select Privet printers on chrome://devices/ and "
"register them."
data:
"Token id for a printer retrieved from a previous request to a "
"Cloud Print Server."
destination: OTHER
}
policy {
cookies_allowed: false
setting: "User triggered requests cannot be disabled."
policy_exception_justification: "Not implemented, it's good to do so."
})");
} else {
DCHECK_EQ(CloudPrintApiFlowRequest::TYPE_SEARCH, type);
return DefineNetworkTrafficAnnotation("cloud_print_search", R"(
semantics {
sender: "Cloud Print"
description:
"Queries a Cloud Print Server for the list of printers."
trigger:
"chrome://devices/ fetches the list when the user logs in,
"re-enable the Cloud Print service, or manually requests a printer "
"list refresh."
data: "None"
destination: OTHER
}
policy {
cookies_allowed: false
setting: "User triggered requests cannot be disabled."
policy_exception_justification: "Not implemented, it's good to do so."
})");
}
}

} // namespace

GCDApiFlowImpl::GCDApiFlowImpl(net::URLRequestContextGetter* request_context,
Expand All @@ -53,7 +101,7 @@ void GCDApiFlowImpl::OnGetTokenSuccess(
const OAuth2TokenService::Request* request,
const std::string& access_token,
const base::Time& expiration_time) {
CreateRequest(request_->GetURL());
CreateRequest();

std::string authorization_header =
base::StringPrintf(kCloudPrintOAuthHeaderFormat, access_token.c_str());
Expand All @@ -70,19 +118,18 @@ void GCDApiFlowImpl::OnGetTokenFailure(
request_->OnGCDApiFlowError(ERROR_TOKEN);
}

void GCDApiFlowImpl::CreateRequest(const GURL& url) {
net::URLFetcher::RequestType request_type = request_->GetRequestType();
DCHECK_EQ(net::URLFetcher::GET, request_type);
void GCDApiFlowImpl::CreateRequest() {
url_fetcher_ = net::URLFetcher::Create(
request_->GetURL(), net::URLFetcher::GET, this,
GetNetworkTrafficAnnotation(request_->GetNetworkTrafficAnnotationType()));
url_fetcher_->SetRequestContext(request_context_.get());

url_fetcher_ = net::URLFetcher::Create(url, request_type, this);
std::vector<std::string> extra_headers = request_->GetExtraRequestHeaders();
for (const std::string& header : extra_headers)
url_fetcher_->AddExtraRequestHeader(header);

data_use_measurement::DataUseUserData::AttachToFetcher(
url_fetcher_.get(), data_use_measurement::DataUseUserData::CLOUD_PRINT);
url_fetcher_->SetRequestContext(request_context_.get());

std::vector<std::string> extra_headers = request_->GetExtraRequestHeaders();
for (size_t i = 0; i < extra_headers.size(); ++i)
url_fetcher_->AddExtraRequestHeader(extra_headers[i]);
}

void GCDApiFlowImpl::OnURLFetchComplete(const net::URLFetcher* source) {
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/printing/cloud_print/gcd_api_flow_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class GCDApiFlowImpl : public GCDApiFlow,
const GoogleServiceAuthError& error) override;

private:
void CreateRequest(const GURL& url);
void CreateRequest();

std::unique_ptr<net::URLFetcher> url_fetcher_;
std::unique_ptr<OAuth2TokenService::Request> oauth_request_;
Expand Down
10 changes: 6 additions & 4 deletions chrome/browser/printing/cloud_print/gcd_api_flow_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ class MockDelegate : public CloudPrintApiFlowRequest {
public:
MOCK_METHOD1(OnGCDApiFlowError, void(GCDApiFlow::Status));
MOCK_METHOD1(OnGCDApiFlowComplete, void(const base::DictionaryValue&));

MOCK_METHOD0(GetURL, GURL());
MOCK_METHOD0(GetNetworkTrafficAnnotationType,
GCDApiFlow::Request::NetworkTrafficAnnotation());
};

class GCDApiFlowTest : public testing::Test {
Expand All @@ -61,15 +62,16 @@ class GCDApiFlowTest : public testing::Test {
token_service_.AddAccount(account_id_);
ui_thread_.Stop(); // HACK: Fake being on the UI thread

std::unique_ptr<MockDelegate> delegate(new MockDelegate);
std::unique_ptr<MockDelegate> delegate = base::MakeUnique<MockDelegate>();
mock_delegate_ = delegate.get();
EXPECT_CALL(*mock_delegate_, GetURL())
.WillRepeatedly(Return(
GURL("https://www.google.com/cloudprint/confirm?token=SomeToken")));
gcd_flow_.reset(new GCDApiFlowImpl(
request_context_.get(), &token_service_, account_id_));
gcd_flow_ = base::MakeUnique<GCDApiFlowImpl>(request_context_.get(),
&token_service_, account_id_);
gcd_flow_->Start(std::move(delegate));
}

base::MessageLoopForUI loop_;
content::TestBrowserThread ui_thread_;
scoped_refptr<net::TestURLRequestContextGetter> request_context_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,14 @@ void PrivetConfirmApiCallFlow::OnGCDApiFlowComplete(
callback_.Run(success ? GCDApiFlow::SUCCESS : GCDApiFlow::ERROR_FROM_SERVER);
}

net::URLFetcher::RequestType PrivetConfirmApiCallFlow::GetRequestType() {
return net::URLFetcher::GET;
}

GURL PrivetConfirmApiCallFlow::GetURL() {
return net::AppendQueryParameter(
cloud_devices::GetCloudPrintRelativeURL("confirm"), "token", token_);
}

GCDApiFlow::Request::NetworkTrafficAnnotation
PrivetConfirmApiCallFlow::GetNetworkTrafficAnnotationType() {
return TYPE_PRIVET_REGISTER;
}

} // namespace cloud_print
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include <string>

#include "base/values.h"
#include "chrome/browser/printing/cloud_print/gcd_api_flow.h"

namespace cloud_print {
Expand All @@ -25,9 +24,8 @@ class PrivetConfirmApiCallFlow : public CloudPrintApiFlowRequest {
// CloudPrintApiFlowRequest implementation:
void OnGCDApiFlowError(GCDApiFlow::Status status) override;
void OnGCDApiFlowComplete(const base::DictionaryValue& value) override;
net::URLFetcher::RequestType GetRequestType() override;

GURL GetURL() override;
NetworkTrafficAnnotation GetNetworkTrafficAnnotationType() override;

private:
ResponseCallback callback_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <set>

#include "base/json/json_reader.h"
#include "base/values.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"

Expand All @@ -33,7 +34,6 @@ TEST(PrivetConfirmApiFlowTest, Params) {
confirmation.GetURL());
EXPECT_EQ("https://www.googleapis.com/auth/cloudprint",
confirmation.GetOAuthScope());
EXPECT_EQ(net::URLFetcher::GET, confirmation.GetRequestType());
EXPECT_FALSE(confirmation.GetExtraRequestHeaders().empty());
}

Expand Down
Loading

0 comments on commit 413a545

Please sign in to comment.