Commit 22abec60 authored by mathp's avatar mathp Committed by Commit bot

[Autofill] Remove GZIP compression since requests are now in proto

GZIP is detrimental.

BUG=580102
TEST=AutofillDownloadManager
TBR=asvitkine

Review URL: https://codereview.chromium.org/1624543002

Cr-Commit-Position: refs/heads/master@{#372103}
parent a7fe8e8f
......@@ -22,7 +22,6 @@
#include "content/public/test/test_utils.h"
#include "net/url_request/test_url_fetcher_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/zlib/google/compression_utils.h"
namespace autofill {
namespace {
......@@ -90,13 +89,6 @@ class WindowedNetworkObserver : public net::TestURLFetcher::DelegateForTests {
DISALLOW_COPY_AND_ASSIGN(WindowedNetworkObserver);
};
// Compresses |data| and returns the result.
std::string Compress(const std::string& data) {
std::string compressed_data;
EXPECT_TRUE(compression::GzipCompress(data, &compressed_data));
return compressed_data;
}
} // namespace
class AutofillServerTest : public InProcessBrowserTest {
......@@ -152,8 +144,7 @@ IN_PROC_BROWSER_TEST_F(AutofillServerTest,
std::string expected_query_string;
ASSERT_TRUE(query.SerializeToString(&expected_query_string));
WindowedNetworkObserver query_network_observer(
Compress(expected_query_string));
WindowedNetworkObserver query_network_observer(expected_query_string);
ui_test_utils::NavigateToURL(
browser(), GURL(std::string(kDataURIPrefix) + kFormHtml));
......@@ -183,8 +174,7 @@ IN_PROC_BROWSER_TEST_F(AutofillServerTest,
std::string expected_upload_string;
ASSERT_TRUE(upload.SerializeToString(&expected_upload_string));
WindowedNetworkObserver upload_network_observer(
Compress(expected_upload_string));
WindowedNetworkObserver upload_network_observer(expected_upload_string);
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
content::SimulateMouseClick(
......@@ -221,8 +211,7 @@ IN_PROC_BROWSER_TEST_F(AutofillServerTest,
std::string expected_query_string;
ASSERT_TRUE(query.SerializeToString(&expected_query_string));
WindowedNetworkObserver query_network_observer(
Compress(expected_query_string));
WindowedNetworkObserver query_network_observer(expected_query_string);
ui_test_utils::NavigateToURL(
browser(), GURL(std::string(kDataURIPrefix) + kFormHtml));
query_network_observer.Wait();
......
......@@ -2159,7 +2159,6 @@
'../third_party/safe_browsing/safe_browsing.gyp:safe_browsing',
'../third_party/webrtc/modules/modules.gyp:desktop_capture',
'../third_party/widevine/cdm/widevine_cdm.gyp:widevine_cdm_version_h',
'../third_party/zlib/google/zip.gyp:compression_utils',
'../ui/accessibility/accessibility.gyp:accessibility_test_support',
'../ui/compositor/compositor.gyp:compositor_test_support',
'../ui/resources/ui_resources.gyp:ui_resources',
......
......@@ -11,7 +11,6 @@
'dependencies': [
'../base/base.gyp:base',
'../base/base.gyp:base_i18n',
'../third_party/zlib/google/zip.gyp:compression_utils',
'../url/url.gyp:url_lib',
],
'include_dirs': [
......
include_rules = [
"+components/compression",
"+components/os_crypt",
"+components/pref_registry",
"+components/rappor",
......
......@@ -193,7 +193,6 @@ source_set("browser") {
"//third_party/libaddressinput:util",
"//third_party/libphonenumber",
"//third_party/re2",
"//third_party/zlib:compression_utils",
"//ui/base",
"//ui/gfx",
"//ui/gfx/geometry",
......@@ -325,7 +324,6 @@ source_set("unit_tests") {
"//testing/gtest",
"//third_party/libaddressinput:util",
"//third_party/libphonenumber",
"//third_party/zlib:compression_utils",
"//ui/base",
"//url",
]
......
......@@ -26,7 +26,6 @@
#include "net/http/http_response_headers.h"
#include "net/http/http_status_code.h"
#include "net/url_request/url_fetcher.h"
#include "third_party/zlib/google/compression_utils.h"
#include "url/gurl.h"
namespace autofill {
......@@ -238,18 +237,6 @@ bool AutofillDownloadManager::StartRequest(
DCHECK(request_context);
GURL request_url = GetRequestUrl(request_data.request_type);
// TODO(crbug.com/580102): Remove the compression step.
std::string compressed_data;
if (!compression::GzipCompress(request_data.payload, &compressed_data)) {
NOTREACHED();
return false;
}
const int compression_ratio = base::checked_cast<int>(
100 * compressed_data.size() / request_data.payload.size());
AutofillMetrics::LogPayloadCompressionRatio(compression_ratio,
request_data.request_type);
// Id is ignored for regular chrome, in unit test id's for fake fetcher
// factory will be 0, 1, 2, ...
net::URLFetcher* fetcher =
......@@ -260,12 +247,11 @@ bool AutofillDownloadManager::StartRequest(
url_fetchers_[fetcher] = request_data;
fetcher->SetAutomaticallyRetryOn5xx(false);
fetcher->SetRequestContext(request_context);
fetcher->SetUploadData("text/proto", compressed_data);
fetcher->SetUploadData("text/proto", request_data.payload);
fetcher->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES |
net::LOAD_DO_NOT_SEND_COOKIES);
// Add Chrome experiment state and GZIP encoding to the request headers.
// Add Chrome experiment state to the request headers.
net::HttpRequestHeaders headers;
headers.SetHeaderIfMissing("content-encoding", "gzip");
variations::AppendVariationHeaders(
fetcher->GetOriginalURL(), driver_->IsOffTheRecord(), false, &headers);
fetcher->SetExtraRequestHeaders(headers.ToString());
......
......@@ -94,7 +94,6 @@ class AutofillDownloadManager : public net::URLFetcherDelegate {
FRIEND_TEST_ALL_PREFIXES(AutofillDownloadManagerTest, QueryAndUploadTest);
FRIEND_TEST_ALL_PREFIXES(AutofillDownloadManagerTest, BackoffLogic_Upload);
FRIEND_TEST_ALL_PREFIXES(AutofillDownloadManagerTest, BackoffLogic_Query);
FRIEND_TEST_ALL_PREFIXES(AutofillDownloadManagerTest, UploadRequestIsGzipped);
struct FormRequestData;
typedef std::list<std::pair<std::string, std::string> > QueryRequestCache;
......
......@@ -21,14 +21,12 @@
#include "components/autofill/core/browser/form_structure.h"
#include "components/autofill/core/browser/test_autofill_driver.h"
#include "components/autofill/core/common/form_data.h"
#include "net/http/http_request_headers.h"
#include "net/http/http_status_code.h"
#include "net/url_request/test_url_fetcher_factory.h"
#include "net/url_request/url_request_status.h"
#include "net/url_request/url_request_test_util.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/zlib/google/compression_utils.h"
using base::ASCIIToUTF16;
......@@ -49,13 +47,6 @@ void FakeOnURLFetchComplete(net::TestURLFetcher* fetcher,
fetcher->delegate()->OnURLFetchComplete(fetcher);
}
// Compresses |data| and returns the result.
std::string Compress(const std::string& data) {
std::string compressed_data;
EXPECT_TRUE(compression::GzipCompress(data, &compressed_data));
return compressed_data;
}
} // namespace
// This tests AutofillDownloadManager. AutofillDownloadManagerTest implements
......@@ -649,131 +640,4 @@ TEST_F(AutofillDownloadManagerTest, CacheQueryTest) {
EXPECT_EQ(responses[0], responses_.front().response);
}
TEST_F(AutofillDownloadManagerTest, QueryRequestIsGzipped) {
// Expected query (uncompressed for visual verification).
AutofillQueryContents query;
query.set_client_version("6.1.1715.1442/en (GGLL)");
AutofillQueryContents::Form* query_form = query.add_form();
query_form->set_signature(14546501144368603154U);
query_form->add_field()->set_signature(239111655U);
query_form->add_field()->set_signature(3763331450U);
query_form->add_field()->set_signature(3494530716U);
std::string expected_query_string;
ASSERT_TRUE(query.SerializeToString(&expected_query_string));
// Create and register factory.
net::TestURLFetcherFactory factory;
FormData form;
FormFieldData field;
field.form_control_type = "text";
field.label = ASCIIToUTF16("username");
field.name = ASCIIToUTF16("username");
form.fields.push_back(field);
field.label = ASCIIToUTF16("First Name");
field.name = ASCIIToUTF16("firstname");
form.fields.push_back(field);
field.label = ASCIIToUTF16("Last Name");
field.name = ASCIIToUTF16("lastname");
form.fields.push_back(field);
FormStructure* form_structure = new FormStructure(form);
ScopedVector<FormStructure> form_structures;
form_structures.push_back(form_structure);
base::HistogramTester histogram;
// Request with id 0.
EXPECT_TRUE(download_manager_.StartQueryRequest(form_structures.get()));
histogram.ExpectUniqueSample("Autofill.ServerQueryResponse",
AutofillMetrics::QUERY_SENT, 1);
// Request payload is gzipped.
net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
ASSERT_TRUE(fetcher);
EXPECT_EQ(Compress(expected_query_string), fetcher->upload_data());
// Proper content-encoding header is defined.
net::HttpRequestHeaders headers;
fetcher->GetExtraRequestHeaders(&headers);
std::string header;
EXPECT_TRUE(headers.GetHeader("content-encoding", &header));
EXPECT_EQ("gzip", header);
// TODO(http://crbug.com/580102) The >100% compression ratio is a known
// problem.
// Expect that the compression is logged.
// NOTE: To get the expected value, run tests with --vmodule=autofill*=1 and
// watch for the VLOG which indicates compression.
histogram.ExpectUniqueSample("Autofill.PayloadCompressionRatio.Query", 133,
1);
}
TEST_F(AutofillDownloadManagerTest, UploadRequestIsGzipped) {
// Expected upload (uncompressed for visual verification).
AutofillUploadContents upload;
upload.set_submission(true);
upload.set_client_version("6.1.1715.1442/en (GGLL)");
upload.set_form_signature(14546501144368603154U);
upload.set_autofill_used(true);
upload.set_data_present("");
std::string expected_upload_string;
ASSERT_TRUE(upload.SerializeToString(&expected_upload_string));
// Create and register factory.
net::TestURLFetcherFactory factory;
FormData form;
FormFieldData field;
field.form_control_type = "text";
field.label = ASCIIToUTF16("username");
field.name = ASCIIToUTF16("username");
form.fields.push_back(field);
field.label = ASCIIToUTF16("First Name");
field.name = ASCIIToUTF16("firstname");
form.fields.push_back(field);
field.label = ASCIIToUTF16("Last Name");
field.name = ASCIIToUTF16("lastname");
form.fields.push_back(field);
FormStructure* form_structure = new FormStructure(form);
ScopedVector<FormStructure> form_structures;
form_structures.push_back(form_structure);
base::HistogramTester histogram;
// Request with id 0.
EXPECT_TRUE(download_manager_.StartUploadRequest(
*(form_structures[0]), true, ServerFieldTypeSet(), std::string(), true));
// Request payload is gzipped.
net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
ASSERT_TRUE(fetcher);
EXPECT_EQ(Compress(expected_upload_string), fetcher->upload_data());
// Proper content-encoding header is defined.
net::HttpRequestHeaders headers;
fetcher->GetExtraRequestHeaders(&headers);
std::string header;
EXPECT_TRUE(headers.GetHeader("content-encoding", &header));
EXPECT_EQ("gzip", header);
// TODO(http://crbug.com/580102) The >100% compression ratio is a known
// problem.
// Expect that the compression is logged.
// NOTE: To get the expected value, run tests with --vmodule=autofill*=1 and
// watch for the VLOG which indicates compression.
histogram.ExpectUniqueSample("Autofill.PayloadCompressionRatio.Upload", 150,
1);
}
} // namespace autofill
......@@ -683,22 +683,6 @@ void AutofillMetrics::LogAutofillFormSubmittedState(
AUTOFILL_FORM_SUBMITTED_STATE_ENUM_SIZE);
}
// static
void AutofillMetrics::LogPayloadCompressionRatio(
int compression_ratio,
AutofillDownloadManager::RequestType type) {
switch (type) {
case AutofillDownloadManager::REQUEST_QUERY:
UMA_HISTOGRAM_PERCENTAGE("Autofill.PayloadCompressionRatio.Query",
compression_ratio);
break;
case AutofillDownloadManager::REQUEST_UPLOAD:
UMA_HISTOGRAM_PERCENTAGE("Autofill.PayloadCompressionRatio.Upload",
compression_ratio);
break;
}
}
AutofillMetrics::FormEventLogger::FormEventLogger(bool is_for_credit_card)
: is_for_credit_card_(is_for_credit_card),
is_server_data_available_(false),
......
......@@ -10,7 +10,6 @@
#include "base/macros.h"
#include "components/autofill/core/browser/autofill_client.h"
#include "components/autofill/core/browser/autofill_download_manager.h"
#include "components/autofill/core/browser/autofill_profile.h"
#include "components/autofill/core/browser/credit_card.h"
#include "components/autofill/core/browser/field_types.h"
......@@ -651,12 +650,6 @@ class AutofillMetrics {
// state of the form.
static void LogAutofillFormSubmittedState(AutofillFormSubmittedState state);
// Log the compression ratio obtained by compressing with gzip. Logs for the
// query or upload request, depending on |type|.
static void LogPayloadCompressionRatio(
int compression_ratio,
AutofillDownloadManager::RequestType type);
// Utility to autofill form events in the relevant histograms depending on
// the presence of server and/or local data.
class FormEventLogger {
......
......@@ -2516,6 +2516,9 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</histogram>
<histogram name="Autofill.PayloadCompressionRatio" units="%">
<obsolete>
Deprecated as of 1/2016, autofill payload compression was removed.
</obsolete>
<owner>mathp@chromium.org</owner>
<summary>
Compression ratio of the query and upload payload that are sent to the
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