Skip to content

Commit

Permalink
[Sync] Make compression from client to server as default.
Browse files Browse the repository at this point in the history
BUG=657911

Review-Url: https://codereview.chromium.org/2703593006
Cr-Commit-Position: refs/heads/master@{#451909}
  • Loading branch information
gangwu authored and Commit bot committed Feb 22, 2017
1 parent 8cc1739 commit a314137
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 80 deletions.
12 changes: 2 additions & 10 deletions components/sync/engine/net/http_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,6 @@ void RecordSyncResponseContentLengthHistograms(

} // namespace

// Enables compression of messages from client to server.
const base::Feature kSyncClientToServerCompression{
"EnableSyncClientToServerCompression", base::FEATURE_DISABLED_BY_DEFAULT};

HttpBridgeFactory::HttpBridgeFactory(
const scoped_refptr<net::URLRequestContextGetter>& request_context_getter,
const NetworkTimeUpdateCallback& network_time_update_callback,
Expand Down Expand Up @@ -247,12 +243,8 @@ void HttpBridge::MakeAsynchronousPost() {
fetch_state_.url_poster->SetExtraRequestHeaders(extra_headers_);

std::string request_to_send;
if (base::FeatureList::IsEnabled(kSyncClientToServerCompression)) {
compression::GzipCompress(request_content_, &request_to_send);
fetch_state_.url_poster->AddExtraRequestHeader("Content-Encoding: gzip");
} else {
request_to_send = request_content_;
}
compression::GzipCompress(request_content_, &request_to_send);
fetch_state_.url_poster->AddExtraRequestHeader("Content-Encoding: gzip");
fetch_state_.url_poster->SetUploadData(content_type_, request_to_send);
RecordSyncRequestContentLengthHistograms(request_to_send.size(),
request_content_.size());
Expand Down
2 changes: 0 additions & 2 deletions components/sync/engine/net/http_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include <string>

#include "base/compiler_specific.h"
#include "base/feature_list.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/synchronization/lock.h"
Expand All @@ -37,7 +36,6 @@ class URLFetcher;
namespace syncer {

class CancelationSignal;
extern const base::Feature kSyncClientToServerCompression;

// A bridge between the syncer and Chromium HTTP layers.
// Provides a way for the sync backend to use Chromium directly for HTTP
Expand Down
67 changes: 4 additions & 63 deletions components/sync/engine/net/http_bridge_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "base/run_loop.h"
#include "base/single_thread_task_runner.h"
#include "base/strings/stringprintf.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/thread.h"
#include "build/build_config.h"
#include "components/sync/base/cancelation_signal.h"
Expand Down Expand Up @@ -226,41 +225,15 @@ TEST_F(MAYBE_SyncHttpBridgeTest, TestMakeSynchronousPostShunted) {
std::string(http_bridge->GetResponseContent()));
}

// Full round-trip test of the HttpBridge, using default UA string and
// no request cookies.
TEST_F(MAYBE_SyncHttpBridgeTest, TestMakeSynchronousPostLiveWithPayload) {
ASSERT_TRUE(test_server_.Start());

scoped_refptr<HttpBridge> http_bridge(BuildBridge());

std::string payload = "this should be echoed back";
GURL echo = test_server_.GetURL("/echo");
http_bridge->SetURL(echo.spec().c_str(), echo.IntPort());
http_bridge->SetPostPayload("application/x-www-form-urlencoded",
payload.length() + 1, payload.c_str());
int os_error = 0;
int response_code = 0;
bool success = http_bridge->MakeSynchronousPost(&os_error, &response_code);
EXPECT_TRUE(success);
EXPECT_EQ(200, response_code);
EXPECT_EQ(0, os_error);

EXPECT_EQ(payload.length() + 1,
static_cast<size_t>(http_bridge->GetResponseContentLength()));
EXPECT_EQ(payload, std::string(http_bridge->GetResponseContent()));
}

// Full round-trip test of the HttpBridge with compressed data, check if the
// data is correctly compressed.
TEST_F(MAYBE_SyncHttpBridgeTest, CompressedRequestPayloadCheck) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(kSyncClientToServerCompression);

ASSERT_TRUE(test_server_.Start());

scoped_refptr<HttpBridge> http_bridge(BuildBridge());

std::string payload = "this should be echoed back";
std::string payload =
"this should be echoed back, this should be echoed back.";
GURL echo = test_server_.GetURL("/echo");
http_bridge->SetURL(echo.spec().c_str(), echo.IntPort());
http_bridge->SetPostPayload("application/x-www-form-urlencoded",
Expand All @@ -272,7 +245,8 @@ TEST_F(MAYBE_SyncHttpBridgeTest, CompressedRequestPayloadCheck) {
EXPECT_EQ(200, response_code);
EXPECT_EQ(0, os_error);

EXPECT_NE(payload.length() + 1,
// Verifying compression, check if the actual payload is compressed correctly.
EXPECT_GT(payload.length(),
static_cast<size_t>(http_bridge->GetResponseContentLength()));
std::string compressed_payload(http_bridge->GetResponseContent(),
http_bridge->GetResponseContentLength());
Expand All @@ -285,9 +259,6 @@ TEST_F(MAYBE_SyncHttpBridgeTest, CompressedRequestPayloadCheck) {
// fields("Content-Encoding" ,"Accept-Encoding" and user agent) are set
// correctly.
TEST_F(MAYBE_SyncHttpBridgeTest, CompressedRequestHeaderCheck) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(kSyncClientToServerCompression);

ASSERT_TRUE(test_server_.Start());

scoped_refptr<HttpBridge> http_bridge(BuildBridge());
Expand Down Expand Up @@ -318,35 +289,6 @@ TEST_F(MAYBE_SyncHttpBridgeTest, CompressedRequestHeaderCheck) {
"%s: %s", net::HttpRequestHeaders::kUserAgent, kUserAgent)));
}

// Full round-trip test of the HttpBridge.
TEST_F(MAYBE_SyncHttpBridgeTest, TestMakeSynchronousPostLiveComprehensive) {
ASSERT_TRUE(test_server_.Start());

scoped_refptr<HttpBridge> http_bridge(BuildBridge());

GURL echo_header = test_server_.GetURL("/echoall");
http_bridge->SetURL(echo_header.spec().c_str(), echo_header.IntPort());

std::string test_payload = "###TEST PAYLOAD###";
http_bridge->SetPostPayload("text/html", test_payload.length() + 1,
test_payload.c_str());

int os_error = 0;
int response_code = 0;
bool success = http_bridge->MakeSynchronousPost(&os_error, &response_code);
EXPECT_TRUE(success);
EXPECT_EQ(200, response_code);
EXPECT_EQ(0, os_error);

std::string response(http_bridge->GetResponseContent(),
http_bridge->GetResponseContentLength());
EXPECT_EQ(std::string::npos, response.find("Cookie:"));
EXPECT_NE(std::string::npos,
response.find(base::StringPrintf(
"%s: %s", net::HttpRequestHeaders::kUserAgent, kUserAgent)));
EXPECT_NE(std::string::npos, response.find(test_payload.c_str()));
}

TEST_F(MAYBE_SyncHttpBridgeTest, TestExtraRequestHeaders) {
ASSERT_TRUE(test_server_.Start());

Expand All @@ -372,7 +314,6 @@ TEST_F(MAYBE_SyncHttpBridgeTest, TestExtraRequestHeaders) {
http_bridge->GetResponseContentLength());

EXPECT_NE(std::string::npos, response.find("fnord"));
EXPECT_NE(std::string::npos, response.find(test_payload.c_str()));
}

TEST_F(MAYBE_SyncHttpBridgeTest, TestResponseHeader) {
Expand Down
6 changes: 1 addition & 5 deletions components/sync/engine_impl/commit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@

#include <utility>

#include "base/feature_list.h"
#include "base/metrics/histogram_macros.h"
#include "base/rand_util.h"
#include "base/trace_event/trace_event.h"
#include "components/sync/base/data_type_histogram.h"
#include "components/sync/engine/net/http_bridge.h"
#include "components/sync/engine_impl/commit_processor.h"
#include "components/sync/engine_impl/commit_util.h"
#include "components/sync/engine_impl/cycle/sync_cycle.h"
Expand Down Expand Up @@ -79,9 +77,7 @@ Commit* Commit::Init(ModelTypeSet requested_types,
commit_message->set_cache_guid(cache_guid);

// Set padding to mitigate CRIME attack.
if (base::FeatureList::IsEnabled(syncer::kSyncClientToServerCompression)) {
commit_message->set_padding(RandASCIIString(kPaddingSize));
}
commit_message->set_padding(RandASCIIString(kPaddingSize));

// Set extensions activity if bookmark commits are present.
ExtensionsActivity::Records extensions_activity_buffer;
Expand Down

0 comments on commit a314137

Please sign in to comment.