Data Reduction Proxy blocking page and resources

Data Reduction Proxy Blocking page and resources

Adds the blocking page and resources for the Data Reduction Proxy
bypass interstitials. This intersitial is shown when the user
loads a resource that returns a data reduction proxy bypass on http.
The user can choose to continue loading the page or go back to the
previous page. If the user accepts loading the page, the intertitial
will not be shown again for another 5 minutes.

BUG=428408
Committed: https://crrev.com/f005db94949dda29ef9273caa01216969a229c01
Cr-Commit-Position: refs/heads/master@{#316621}

[megjablon, 2015-01-02 23:24:05]

Patchset #1 (id:1) has been deleted

[megjablon, 2015-01-07 23:28:57]

Patchset #1 (id:20001) has been deleted

[megjablon, 2015-01-07 23:29:05]

Patchset #1 (id:40001) has been deleted

[megjablon, 2015-01-08 00:15:20]

Patchset #1 (id:60001) has been deleted

[megjablon, 2015-01-08 00:23:11]

Patchset #1 (id:80001) has been deleted

[megjablon, 2015-01-08 00:43:29]

Patchset #1 (id:100001) has been deleted

[megjablon, 2015-01-08 00:47:25]

megjablon@chromium.org changed reviewers:
+ bengr@chromium.org, blundell@chromium.org

[megjablon, 2015-01-08 00:47:25]

bengr@chromium.org: components/data_reduction_proxy/*

blundell@chromium.org: All files not in components/data_reduction_proxy/*

[blundell, 2015-01-08 08:13:17]

Holding off on reviewing until the discussion that I added you to offline is
resolved.

[megjablon, 2015-01-08 23:05:00]

Patchset #3 (id:160001) has been deleted

[megjablon, 2015-01-14 01:55:00]

Patchset #4 (id:200001) has been deleted

[megjablon, 2015-01-14 01:55:04]

Patchset #3 (id:180001) has been deleted

[megjablon, 2015-01-14 01:55:10]

Patchset #2 (id:140001) has been deleted

[blundell, 2015-01-14 20:52:12]

https://codereview.chromium.org/830503004/diff/220001/components/BUILD.gn
File components/BUILD.gn (right):

https://codereview.chromium.org/830503004/diff/220001/components/BUILD.gn#new...
components/BUILD.gn:124: if (is_ios) {
The components_unittests target should have a block like this one that removes
your new test target as a dep, since it shouldn't run on iOS.

https://codereview.chromium.org/830503004/diff/220001/components/components_s...
File components/components_strings.grd (right):

https://codereview.chromium.org/830503004/diff/220001/components/components_s...
components/components_strings.grd:173: <part
file="data_reduction_proxy_strings.grdp" />
Could you coordinate with agable@ re: making sure that these strings get
included in the set sent for translation?

https://codereview.chromium.org/830503004/diff/220001/components/data_reducti...
File components/data_reduction_proxy/content/browser/BUILD.gn (right):

https://codereview.chromium.org/830503004/diff/220001/components/data_reducti...
components/data_reduction_proxy/content/browser/BUILD.gn:30:
"data_reduction_proxy_blocking_page_unittest.cc",
should this be added only on android, based on the gypfile?

[agable, 2015-01-14 23:07:13]

agable@chromium.org changed reviewers:
+ agable@chromium.org

[agable, 2015-01-14 23:07:14]

https://codereview.chromium.org/830503004/diff/220001/components/components_s...
File components/components_strings.grd (right):

https://codereview.chromium.org/830503004/diff/220001/components/components_s...
components/components_strings.grd:173: <part
file="data_reduction_proxy_strings.grdp" />
On 2015/01/14 20:52:12, blundell wrote:
> Could you coordinate with agable@ re: making sure that these strings get
> included in the set sent for translation?

cl/83977257

[megjablon, 2015-01-15 00:52:00]

https://codereview.chromium.org/830503004/diff/220001/components/BUILD.gn
File components/BUILD.gn (right):

https://codereview.chromium.org/830503004/diff/220001/components/BUILD.gn#new...
components/BUILD.gn:124: if (is_ios) {
On 2015/01/14 20:52:11, blundell wrote:
> The components_unittests target should have a block like this one that removes
> your new test target as a dep, since it shouldn't run on iOS.

Done.

https://codereview.chromium.org/830503004/diff/220001/components/data_reducti...
File components/data_reduction_proxy/content/browser/BUILD.gn (right):

https://codereview.chromium.org/830503004/diff/220001/components/data_reducti...
components/data_reduction_proxy/content/browser/BUILD.gn:30:
"data_reduction_proxy_blocking_page_unittest.cc",
On 2015/01/14 20:52:12, blundell wrote:
> should this be added only on android, based on the gypfile?

Done.

[blundell, 2015-01-15 01:07:18]

lgtm

[agable, 2015-01-21 23:05:57]

On 2015/01/15 at 01:07:18, blundell wrote:
> lgtm

Please land this so I can land cl/83977257

[megjablon, 2015-01-21 23:09:40]

On 2015/01/21 23:05:57, agable wrote:
> On 2015/01/15 at 01:07:18, blundell wrote:
> > lgtm
> 
> Please land this so I can land cl/83977257

It's going to be a little while before this lands.

There are two CLs that need to land before this one can:
https://codereview.chromium.org/684223003/
https://codereview.chromium.org/778463002/

[agable, 2015-01-22 21:20:08]

On 2015/01/21 at 23:09:40, megjablon wrote:
> On 2015/01/21 23:05:57, agable wrote:
> > On 2015/01/15 at 01:07:18, blundell wrote:
> > > lgtm
> > 
> > Please land this so I can land cl/83977257
> 
> It's going to be a little while before this lands.
> 
> There are two CLs that need to land before this one can:
> https://codereview.chromium.org/684223003/
> https://codereview.chromium.org/778463002/

Ah, ok, no problem then. I'll keep an eye on this and land the google3-side CL
when this does.

[megjablon, 2015-02-04 19:36:46]

Patchset #6 (id:300001) has been deleted

[bengr, 2015-02-05 01:30:10]

Looks pretty good.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/content_data_reduction_proxy_debug_ui_service.cc
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/content_data_reduction_proxy_debug_ui_service.cc:17:
const std::string& app_locale)
#include <string>

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc:27:

Remove blank line.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc:34:

Remove blank line.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc:38:
DataReductionProxyDebugBlockingPageFactory*
I suppose this is how other interstitials are written?

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc:54:
return new DataReductionProxyDebugBlockingPage(ui_manager, io_task_runner,
If they'll all fit on one line, move ui_manager down to the next line.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc:180:
resource_list_, false);
indentation.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc:188:
iter->second, false);
indentation.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:30:
// When a user is about to visit a page and the the Data Reduction Proxy
the the -> the

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:31:
// bypasses this class show an interstitial page with some options (go back,
class -> class,

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:46:
// in the first interstitial, we will not display another intersitial for a
Remove "we" from comments.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:67:
// specific resource. You can call this method several times, if an
This method can be called several times...

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:78:
// Makes the passed |factory| the factory used to instantiate
Makes |factory| the...

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:85:
// InterstitialPageDelegate methods:
methods -> overrides

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:101:
DataReductionProxyDebugBlockingPage(
Can this be private?

Also say "This constructor should not be called directly. Use Show..."

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:151:
content::InterstitialPage* interstitial_page_;  // Owns us
Who is us? Don't personify code.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:159:
// Whether the interstitial should create a view.
Comments should be complete sentences.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:162:
const std::string& app_locale_;
Add comments for all member variables.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:164:
DISALLOW_COPY_AND_ASSIGN(DataReductionProxyDebugBlockingPage);
#include base/macros.h

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:167:
// Factory for creating DataReductionProxyDebugBlockingPage.  Useful for tests.
Why not move this to the tests?

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc:129:
content::RenderFrameHost* rfh = is_cross_site ?
render_frame_host, not rfh

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc:418:
// cause problems.
What kinds of problems?

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc:425:
ShowInterstitial(false, kBypassURL);
These tests don't test that the (actual or simulated) reception of a bypass
message triggers the interstitial. Please add a TODO to add such tests.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_resource_throttle.cc
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_resource_throttle.cc:68:
bool* defer) {
indentation.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_ui_manager.cc
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_ui_manager.cc:100:
iter != resources.end(); ++iter) {
I'd indent one more.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_ui_manager.h
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_ui_manager.h:51:
const std::string& app_locale);
Can local be empty? Say so and say how it will be used.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File components/data_reduction_proxy/content/resources/interstitial.html
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/resources/interstitial.html:1: <!DOCTYPE
html>
Do we put a copyright notice on html?

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File components/data_reduction_proxy_strings.grdp (right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy_strings.grdp:18: This page is unproxyable by
Data Saver.
Is "unproxyable" a word? How about "This page cannot be proxied by Data Saver."

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy_strings.grdp:21: You are trying to reach <ph
name="BEGIN_BOLD">&lt;strong&gt;</ph><ph
name="SITE">$1<ex>example.com</ex></ph><ph name="END_BOLD">&lt;/strong&gt;</ph>.
This page is unproxyable by Data Saver at this time.
See above about unproxyable.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy_strings.grdp:24: You may continue. If you
continue to the page, this warning will not appear again for five minutes.
What if you don't continue? If you don't continue, will the next bypass trigger
the interstitial again?

https://codereview.chromium.org/830503004/diff/320001/components/resources/OW...
File components/resources/OWNERS (right):

https://codereview.chromium.org/830503004/diff/320001/components/resources/OW...
components/resources/OWNERS:3: per-file data_reduction_proxy*=bengr@chromium.org
Add scott here too.

[megjablon, 2015-02-13 01:47:54]

Patchset #8 (id:360001) has been deleted

[megjablon, 2015-02-13 01:48:09]

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/content_data_reduction_proxy_debug_ui_service.cc
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/content_data_reduction_proxy_debug_ui_service.cc:17:
const std::string& app_locale)
On 2015/02/05 01:30:08, bengr wrote:
> #include <string>

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc:27:

On 2015/02/05 01:30:08, bengr wrote:
> Remove blank line.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc:34:

On 2015/02/05 01:30:08, bengr wrote:
> Remove blank line.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc:38:
DataReductionProxyDebugBlockingPageFactory*
On 2015/02/05 01:30:08, bengr wrote:
> I suppose this is how other interstitials are written?

Yep. This is how safe browsing is.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc:54:
return new DataReductionProxyDebugBlockingPage(ui_manager, io_task_runner,
On 2015/02/05 01:30:08, bengr wrote:
> If they'll all fit on one line, move ui_manager down to the next line.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc:180:
resource_list_, false);
On 2015/02/05 01:30:08, bengr wrote:
> indentation.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.cc:188:
iter->second, false);
On 2015/02/05 01:30:08, bengr wrote:
> indentation.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:30:
// When a user is about to visit a page and the the Data Reduction Proxy
On 2015/02/05 01:30:08, bengr wrote:
> the the -> the

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:31:
// bypasses this class show an interstitial page with some options (go back,
On 2015/02/05 01:30:09, bengr wrote:
> class -> class,

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:46:
// in the first interstitial, we will not display another intersitial for a
On 2015/02/05 01:30:09, bengr wrote:
> Remove "we" from comments.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:67:
// specific resource. You can call this method several times, if an
On 2015/02/05 01:30:09, bengr wrote:
> This method can be called several times...

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:78:
// Makes the passed |factory| the factory used to instantiate
On 2015/02/05 01:30:08, bengr wrote:
> Makes |factory| the...

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:85:
// InterstitialPageDelegate methods:
On 2015/02/05 01:30:09, bengr wrote:
> methods -> overrides

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:101:
DataReductionProxyDebugBlockingPage(
On 2015/02/05 01:30:09, bengr wrote:
> Can this be private?
> 
> Also say "This constructor should not be called directly. Use Show..."

Can't be private because the factory uses it.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:151:
content::InterstitialPage* interstitial_page_;  // Owns us
On 2015/02/05 01:30:08, bengr wrote:
> Who is us? Don't personify code.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:159:
// Whether the interstitial should create a view.
On 2015/02/05 01:30:09, bengr wrote:
> Comments should be complete sentences.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:162:
const std::string& app_locale_;
On 2015/02/05 01:30:09, bengr wrote:
> Add comments for all member variables.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:164:
DISALLOW_COPY_AND_ASSIGN(DataReductionProxyDebugBlockingPage);
On 2015/02/05 01:30:09, bengr wrote:
> #include base/macros.h

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:167:
// Factory for creating DataReductionProxyDebugBlockingPage.  Useful for tests.
On 2015/02/05 01:30:09, bengr wrote:
> Why not move this to the tests?

As commented, this is how the other interstitials are written.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc:129:
content::RenderFrameHost* rfh = is_cross_site ?
On 2015/02/05 01:30:09, bengr wrote:
> render_frame_host, not rfh

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc:418:
// cause problems.
On 2015/02/05 01:30:09, bengr wrote:
> What kinds of problems?

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc:425:
ShowInterstitial(false, kBypassURL);
On 2015/02/05 01:30:09, bengr wrote:
> These tests don't test that the (actual or simulated) reception of a bypass
> message triggers the interstitial. Please add a TODO to add such tests.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_resource_throttle.cc
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_resource_throttle.cc:68:
bool* defer) {
On 2015/02/05 01:30:09, bengr wrote:
> indentation.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_ui_manager.cc
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_ui_manager.cc:100:
iter != resources.end(); ++iter) {
On 2015/02/05 01:30:09, bengr wrote:
> I'd indent one more.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_ui_manager.h
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_ui_manager.h:51:
const std::string& app_locale);
On 2015/02/05 01:30:09, bengr wrote:
> Can local be empty? Say so and say how it will be used.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File components/data_reduction_proxy/content/resources/interstitial.html
(right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy/content/resources/interstitial.html:1: <!DOCTYPE
html>
On 2015/02/05 01:30:09, bengr wrote:
> Do we put a copyright notice on html?

Nope.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
File components/data_reduction_proxy_strings.grdp (right):

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy_strings.grdp:18: This page is unproxyable by
Data Saver.
On 2015/02/05 01:30:09, bengr wrote:
> Is "unproxyable" a word? How about "This page cannot be proxied by Data
Saver."

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy_strings.grdp:21: You are trying to reach <ph
name="BEGIN_BOLD">&lt;strong&gt;</ph><ph
name="SITE">$1<ex>example.com</ex></ph><ph name="END_BOLD">&lt;/strong&gt;</ph>.
This page is unproxyable by Data Saver at this time.
On 2015/02/05 01:30:09, bengr wrote:
> See above about unproxyable.

Done.

https://codereview.chromium.org/830503004/diff/320001/components/data_reducti...
components/data_reduction_proxy_strings.grdp:24: You may continue. If you
continue to the page, this warning will not appear again for five minutes.
On 2015/02/05 01:30:09, bengr wrote:
> What if you don't continue? If you don't continue, will the next bypass
trigger
> the interstitial again?

Yep. As long as you don't say you're ok with bypassing, then we warn you every
time a bypass happens.

https://codereview.chromium.org/830503004/diff/320001/components/resources/OW...
File components/resources/OWNERS (right):

https://codereview.chromium.org/830503004/diff/320001/components/resources/OW...
components/resources/OWNERS:3: per-file data_reduction_proxy*=bengr@chromium.org
On 2015/02/05 01:30:09, bengr wrote:
> Add scott here too.

Done.

[bengr, 2015-02-13 16:34:33]

lgtm with nits.

https://codereview.chromium.org/830503004/diff/380001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h
(right):

https://codereview.chromium.org/830503004/diff/380001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page.h:158:
// Owns this DataReductionproxyDebugBlockingPage
Add a period.

https://codereview.chromium.org/830503004/diff/380001/components/data_reducti...
File
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc
(right):

https://codereview.chromium.org/830503004/diff/380001/components/data_reducti...
components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc:73:
// TODO(megajblon): Add tests that verify the reception of a bypass message
Spell your username correctly. :)

[megjablon, 2015-02-13 20:10:48]

New patchsets have been uploaded after l-g-t-m from bengr@chromium.org

[sadrul, 2015-02-14 00:35:44]

sadrul@chromium.org changed reviewers:
+ sadrul@chromium.org

[sadrul, 2015-02-14 00:35:45]

DEPS on ui/base/ lgtm

[megjablon, 2015-02-14 00:46:46]

Patchset #14 (id:500001) has been deleted

[megjablon, 2015-02-14 00:46:52]

New patchsets have been uploaded after l-g-t-m from sadrul@chromium.org

[megjablon, 2015-02-14 00:55:47]

Patchset #14 (id:520001) has been deleted

[megjablon, 2015-02-14 01:00:40]

Patchset #13 (id:480001) has been deleted

[scottmg, 2015-02-14 01:03:32]

scottmg@chromium.org changed reviewers:
+ scottmg@chromium.org

[scottmg, 2015-02-14 01:03:33]

build/ lgtm

[megjablon, 2015-02-14 01:05:10]

Patchset #13 (id:540001) has been deleted

[megjablon, 2015-02-14 01:05:15]

Patchset #12 (id:460001) has been deleted

[megjablon, 2015-02-14 03:21:33]

New patchsets have been uploaded after l-g-t-m from scottmg@chromium.org

[megjablon, 2015-02-14 04:18:03]

Patchset #13 (id:580001) has been deleted

[megjablon, 2015-02-17 19:04:30]

The CQ bit was checked by megjablon@chromium.org

[commit-bot: I haz the power, 2015-02-17 19:06:01]

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

[commit-bot: I haz the power, 2015-02-17 19:10:07]

Committed patchset #13 (id:600001)

[commit-bot: I haz the power, 2015-02-17 19:10:50]

Patchset 13 (id:??) landed as
https://crrev.com/f005db94949dda29ef9273caa01216969a229c01
Cr-Commit-Position: refs/heads/master@{#316621}