[DRP] Consistently use LoFi for an entire page

components/data_reduction_proxy/core/common/data_reduction_proxy_lofi_helper.h

Index: components/data_reduction_proxy/core/common/data_reduction_proxy_lofi_helper.h
diff --git a/components/data_reduction_proxy/core/common/data_reduction_proxy_lofi_helper.h b/components/data_reduction_proxy/core/common/data_reduction_proxy_lofi_helper.h
new file mode 100644
index 0000000000000000000000000000000000000000..d445b800d3729862be1127a8ce42ff23385938e0
--- /dev/null
+++ b/components/data_reduction_proxy/core/common/data_reduction_proxy_lofi_helper.h
@@ -0,0 +1,27 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_DATA_REDUCTION_PROXY_CORE_BROWSER_DATA_REDUCTION_PROXY_LOFI_HELPER_H_
+#define COMPONENTS_DATA_REDUCTION_PROXY_CORE_BROWSER_DATA_REDUCTION_PROXY_LOFI_HELPER_H_
+
+#include "base/macros.h"
+#include "net/url_request/url_request.h"
+
+namespace data_reduction_proxy {
+
+// An interface for the ContentDataReductionProxyLoFiHelper which creates

[bengr, 2015/10/03 00:01:30]

This comment seems to be a layering violation. The interface shouldn't specify
its subclass.

// Interface to determine if a request should be made for a low fidelity
// version of the resource.

[megjablon, 2015/10/05 20:22:58]

Done.

+// a DataReductionProxyLoFiHelper that handles getting if a request is in Lo-Fi
+// mode.
+class DataReductionProxyLoFiHelper {

[bengr, 2015/10/03 00:01:30]

I'd really, really like to drop the DataReductionProxy prefix from all of our
class names, because we already have the namespace.

How about LoFiDecider?

[megjablon, 2015/10/05 20:22:58]

Done.

+ public:
+  virtual ~DataReductionProxyLoFiHelper() {}
+
+  // Returns true if the Lo-Fi directive should be added to the Chrome-Proxy

[bengr, 2015/10/03 00:01:30]

Why do we need a class? Couldn't this just be an injected callback?

[megjablon, 2015/10/05 20:22:58]

We can't inject a callback into OnBeforeSendProxyHeadersInternal, which is where
this is used, but we could have DataReductionProxyIOData own a callback that is
created by DRPChromeIOData. That won't change much of this code, and it just
seems cleaner to me to have this in its own class rather then have
DRPChromeIOData create a callback for some logic that it knows nothing about.

+  // header for the given request.
+  virtual bool ShouldUseLoFi(const net::URLRequest* request) const = 0;

[bengr, 2015/10/03 00:01:30]

I understand why you have this interface, but the text is completely confusing.
So is it:

// Returns true if a low fidelity version of the resource should be requested.

Also, the behavior is undefined for a null request, so I would use a const&
instead.

Finally, why not just have the helper add the header?

[megjablon, 2015/10/05 20:22:58]

Done.
Done.
This sounds good to me, but this cl is already large and I don't want to move
around too much code at once. Can we make this a follow up cl? This would
require pulling all the logic and tests from DRPRequestOptions.

+};
+
+}  // namespace data_reduction_proxy
+
+#endif  // COMPONENTS_DATA_REDUCTION_PROXY_CORE_BROWSER_DATA_REDUCTION_PROXY_LOFI_HELPER_H_