Tests for "Save Image As..." when Data Saver is enabled.

Tests for "Save Image As..." when Data Saver is enabled.

Verify that request headers specify a passthrough when "Save Image As..."
feature is used with Data Saver enabled, and this header is not specified
with Data Saver disabled.

Override SaveFrameWithHeaders in TestWebContents to keep track of
the headers.

BUG=467163
Committed: https://crrev.com/6db683ca806cdf9b78deea233d6b45794fc655c1
Cr-Commit-Position: refs/heads/master@{#329900}

[Not at Google. Contact bengr, 2015-05-07 22:51:34]

kundaji@chromium.org changed reviewers:
+ bengr@chromium.org

[Not at Google. Contact bengr, 2015-05-07 22:53:10]

Ben, Can you please take a look at the changes in data reduction code before I
send this to context menu owners?

The changes are required to enable using command line to simulate feature being
on without initializing all of the state.

[Not at Google. Contact bengr, 2015-05-11 20:01:12]

kundaji@chromium.org changed reviewers:
+ avi@chromium.org
- bengr@chromium.org

[Not at Google. Contact bengr, 2015-05-11 20:01:13]

avi:
  chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc
  content/public/test/web_contents_tester.h
  content/test/test_web_contents.cc
  content/test/test_web_contents.h

[Avi (use Gerrit), 2015-05-11 20:52:29]

https://codereview.chromium.org/1137563002/diff/60001/chrome/browser/renderer...
File chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc
(right):

https://codereview.chromium.org/1137563002/diff/60001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc:457:
.Build();
Can you abstract some of this duplicate code out to a helper function?

https://codereview.chromium.org/1137563002/diff/60001/content/public/test/web...
File content/public/test/web_contents_tester.h (right):

https://codereview.chromium.org/1137563002/diff/60001/content/public/test/web...
content/public/test/web_contents_tester.h:8: #include <string>
Space after this line.

https://codereview.chromium.org/1137563002/diff/60001/content/test/test_web_c...
File content/test/test_web_contents.cc (right):

https://codereview.chromium.org/1137563002/diff/60001/content/test/test_web_c...
content/test/test_web_contents.cc:35: save_frame_headers_(std::string()) {
You don't need to initialize objects that initialize themselves.

[Not at Google. Contact bengr, 2015-05-11 22:43:14]

Thanks for the review. PTAL.

https://codereview.chromium.org/1137563002/diff/60001/chrome/browser/renderer...
File chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc
(right):

https://codereview.chromium.org/1137563002/diff/60001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc:457:
.Build();
On 2015/05/11 20:52:29, Avi wrote:
> Can you abstract some of this duplicate code out to a helper function?

Done.

https://codereview.chromium.org/1137563002/diff/60001/content/public/test/web...
File content/public/test/web_contents_tester.h (right):

https://codereview.chromium.org/1137563002/diff/60001/content/public/test/web...
content/public/test/web_contents_tester.h:8: #include <string>
On 2015/05/11 20:52:29, Avi wrote:
> Space after this line.

Done.

https://codereview.chromium.org/1137563002/diff/60001/content/test/test_web_c...
File content/test/test_web_contents.cc (right):

https://codereview.chromium.org/1137563002/diff/60001/content/test/test_web_c...
content/test/test_web_contents.cc:35: save_frame_headers_(std::string()) {
On 2015/05/11 20:52:29, Avi wrote:
> You don't need to initialize objects that initialize themselves.

Done.

[Avi (use Gerrit), 2015-05-12 00:43:33]

The code LGTM; get a review by an area expert too.

[Not at Google. Contact bengr, 2015-05-12 16:16:51]

kundaji@chromium.org changed reviewers:
+ bengr@chromium.org

[Not at Google. Contact bengr, 2015-05-12 16:24:26]

kundaji@chromium.org changed reviewers:
+ jeremyim@chromium.org
- bengr@chromium.org

[Not at Google. Contact bengr, 2015-05-12 16:24:27]

Thanks avi@.

Jeremy, can you please take a look?

[Not at Google. Contact bengr, 2015-05-12 17:40:09]

kundaji@chromium.org changed reviewers:
+ bengr@chromium.org
- jeremyim@chromium.org

[Not at Google. Contact bengr, 2015-05-12 17:40:27]

Ben, can you please take a look?

[bengr, 2015-05-13 17:35:48]

https://codereview.chromium.org/1137563002/diff/80001/chrome/browser/renderer...
File chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc
(right):

https://codereview.chromium.org/1137563002/diff/80001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc:384:
data_reduction_proxy::DataReductionProxyIOData* io_data =
No need for this temporary.

https://codereview.chromium.org/1137563002/diff/80001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc:390:
base::MessageLoopProxy::current());
#include "base/message_loop/message_loop_proxy.h"

https://codereview.chromium.org/1137563002/diff/80001/content/public/test/web...
File content/public/test/web_contents_tester.h (right):

https://codereview.chromium.org/1137563002/diff/80001/content/public/test/web...
content/public/test/web_contents_tester.h:106: // Returns headers which were
passed in the previous SaveFrameWithHeaders(...)
nit: which -> that

https://codereview.chromium.org/1137563002/diff/80001/content/test/test_web_c...
File content/test/test_web_contents.h (right):

https://codereview.chromium.org/1137563002/diff/80001/content/test/test_web_c...
content/test/test_web_contents.h:54: const std::string& GetSaveFrameHeaders()
override;
#include <string>

https://codereview.chromium.org/1137563002/diff/80001/content/test/test_web_c...
content/test/test_web_contents.h:120: void SaveFrameWithHeaders(const GURL& url,
Forward declare.

https://codereview.chromium.org/1137563002/diff/80001/content/test/test_web_c...
content/test/test_web_contents.h:121: const Referrer& referrer,
Forward declare.

[Not at Google. Contact bengr, 2015-05-13 20:24:09]

https://codereview.chromium.org/1137563002/diff/80001/chrome/browser/renderer...
File chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc
(right):

https://codereview.chromium.org/1137563002/diff/80001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc:384:
data_reduction_proxy::DataReductionProxyIOData* io_data =
On 2015/05/13 17:35:47, bengr wrote:
> No need for this temporary.

Done.

https://codereview.chromium.org/1137563002/diff/80001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc:390:
base::MessageLoopProxy::current());
On 2015/05/13 17:35:47, bengr wrote:
> #include "base/message_loop/message_loop_proxy.h"

Done.

https://codereview.chromium.org/1137563002/diff/80001/content/public/test/web...
File content/public/test/web_contents_tester.h (right):

https://codereview.chromium.org/1137563002/diff/80001/content/public/test/web...
content/public/test/web_contents_tester.h:106: // Returns headers which were
passed in the previous SaveFrameWithHeaders(...)
On 2015/05/13 17:35:47, bengr wrote:
> nit: which -> that

Done.

https://codereview.chromium.org/1137563002/diff/80001/content/test/test_web_c...
File content/test/test_web_contents.h (right):

https://codereview.chromium.org/1137563002/diff/80001/content/test/test_web_c...
content/test/test_web_contents.h:54: const std::string& GetSaveFrameHeaders()
override;
On 2015/05/13 17:35:47, bengr wrote:
> #include <string>

Done.

https://codereview.chromium.org/1137563002/diff/80001/content/test/test_web_c...
content/test/test_web_contents.h:120: void SaveFrameWithHeaders(const GURL& url,
On 2015/05/13 17:35:47, bengr wrote:
> Forward declare.

Done.

https://codereview.chromium.org/1137563002/diff/80001/content/test/test_web_c...
content/test/test_web_contents.h:121: const Referrer& referrer,
On 2015/05/13 17:35:47, bengr wrote:
> Forward declare.

Done.

[bengr, 2015-05-14 16:11:00]

lgtm

[Not at Google. Contact bengr, 2015-05-14 18:33:07]

The CQ bit was checked by kundaji@chromium.org

[Not at Google. Contact bengr, 2015-05-14 18:33:08]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org,
bengr@chromium.org
Link to the patchset: https://codereview.chromium.org/1137563002/#ps160001
(title: "Sync to head")

[commit-bot: I haz the power, 2015-05-14 18:34:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1137563002/160001

[commit-bot: I haz the power, 2015-05-14 19:40:24]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-05-14 19:41:12]

Patchset 9 (id:??) landed as
https://crrev.com/6db683ca806cdf9b78deea233d6b45794fc655c1
Cr-Commit-Position: refs/heads/master@{#329900}