New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 796952 link

Starred by 2 users
Status: Fixed
Owner:
dougarnett@chromium.org
Closed: Jan 2018
Cc:
sclit...@chromium.org
bengr@chromium.org
█ manisca...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Task

Blocking:
issue 783372
issue 799653
issue 801275



Sign in to add a comment

Don't perform client previews (ie, NoScript nor Client LoFi) if "Cache-Control: no-transform" header on main response

Project Member Reported by dougarnett@chromium.org, Dec 21 2017 
This blocks adding NoScript Preview to Beta experiment.

Comment 1 by dougarnett@chromium.org, Jan 5 2018

Status: Started (was: Assigned)

Comment 2 by dougarnett@chromium.org, Jan 6 2018

Blocking: 799653

Comment 3 by tbansal@chromium.org, Jan 9 2018 

Does this bug also tracks the work for checking for "Cache-Control: no-transform" header on the HTTP *request* header (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control)?

Comment 4 by dougarnett@chromium.org, Jan 9 2018 

tbansal: No, please open a separate bug for request directive

Comment 5 by dougarnett@chromium.org, Jan 9 2018

Summary: Don't perform client previews (ie, NoScript nor Client LoFi) if "Cache-Control: no-transform" header on main response (was: Don't perform NoScript preview if "Cache-Control: no-transform" header on main response)
Generalizing this bug to cover Client LoFi as well (to also support weblite
search results telling us to not intervene).

The proposed model is that the client honors no-transform in main frame response
for client previews. The data saver proxy should handle honoring the directive for server-side previews.

Comment 6 by dougarnett@chromium.org, Jan 10 2018

Cc: sclit...@chromium.org
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 11 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/635606bad248ca3644c8039402d8debd769fdd82

commit 635606bad248ca3644c8039402d8debd769fdd82
Author: Doug Arnett <dougarnett@chromium.org>
Date: Thu Jan 11 00:21:09 2018

Considers Cache-Control:no-transform header for committed PreviewsState

Sets PreviewsState to PREVIEWS_OFF if main frame response has the
no-transform header for an otherwise eligible client preview.

Bug:  796952 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ic0a026c340c54529f3a8bd796599fb2aaab9e482
Reviewed-on: https://chromium-review.googlesource.com/853122
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528500}
[modify] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/chrome/browser/loader/chrome_resource_dispatcher_host_delegate_unittest.cc
[modify] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/chrome/browser/previews/previews_browsertest.cc
[modify] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/chrome/browser/previews/previews_infobar_tab_helper.cc
[add] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/chrome/test/data/previews/noscript_test_with_no_transform_header.html
[add] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/chrome/test/data/previews/noscript_test_with_no_transform_header.html.mock-http-headers
[modify] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.cc
[modify] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/components/previews/content/previews_content_util.cc
[modify] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/components/previews/content/previews_content_util.h
[modify] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/components/previews/content/previews_content_util_unittest.cc
[modify] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/components/previews/core/previews_user_data.cc
[modify] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/components/previews/core/previews_user_data.h
[modify] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
[modify] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/635606bad248ca3644c8039402d8debd769fdd82/tools/metrics/histograms/histograms.xml
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 11 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/16a167f0178878c08c10ab9a1a81af6357df5e12

commit 16a167f0178878c08c10ab9a1a81af6357df5e12
Author: Doug Arnett <dougarnett@chromium.org>
Date: Thu Jan 11 17:23:27 2018

Revert "Considers Cache-Control:no-transform header for committed PreviewsState"

This reverts commit 635606bad248ca3644c8039402d8debd769fdd82.

Reason for revert: Causing crashes in ChromeResourceDispatcherHostDelegate::OnResponseStarted

Original change's description:
> Considers Cache-Control:no-transform header for committed PreviewsState
> 
> Sets PreviewsState to PREVIEWS_OFF if main frame response has the
> no-transform header for an otherwise eligible client preview.
> 
> Bug:  796952 
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
> Change-Id: Ic0a026c340c54529f3a8bd796599fb2aaab9e482
> Reviewed-on: https://chromium-review.googlesource.com/853122
> Reviewed-by: Steven Holte <holte@chromium.org>
> Reviewed-by: Charlie Harrison <csharrison@chromium.org>
> Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
> Commit-Queue: Doug Arnett <dougarnett@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#528500}

TBR=holte@chromium.org,csharrison@chromium.org,dougarnett@chromium.org,ryansturm@chromium.org

Change-Id: I6aaf3651434caf3f5ff93ff7a0edabb9722187ca
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  796952 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/862022
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528656}
[modify] https://crrev.com/16a167f0178878c08c10ab9a1a81af6357df5e12/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/16a167f0178878c08c10ab9a1a81af6357df5e12/chrome/browser/loader/chrome_resource_dispatcher_host_delegate_unittest.cc
[modify] https://crrev.com/16a167f0178878c08c10ab9a1a81af6357df5e12/chrome/browser/previews/previews_browsertest.cc
[modify] https://crrev.com/16a167f0178878c08c10ab9a1a81af6357df5e12/chrome/browser/previews/previews_infobar_tab_helper.cc
[delete] https://crrev.com/ebe35c454f68a40334f4e5d8186c51de328d5069/chrome/test/data/previews/noscript_test_with_no_transform_header.html
[delete] https://crrev.com/ebe35c454f68a40334f4e5d8186c51de328d5069/chrome/test/data/previews/noscript_test_with_no_transform_header.html.mock-http-headers
[modify] https://crrev.com/16a167f0178878c08c10ab9a1a81af6357df5e12/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.cc
[modify] https://crrev.com/16a167f0178878c08c10ab9a1a81af6357df5e12/components/previews/content/previews_content_util.cc
[modify] https://crrev.com/16a167f0178878c08c10ab9a1a81af6357df5e12/components/previews/content/previews_content_util.h
[modify] https://crrev.com/16a167f0178878c08c10ab9a1a81af6357df5e12/components/previews/content/previews_content_util_unittest.cc
[modify] https://crrev.com/16a167f0178878c08c10ab9a1a81af6357df5e12/components/previews/core/previews_user_data.cc
[modify] https://crrev.com/16a167f0178878c08c10ab9a1a81af6357df5e12/components/previews/core/previews_user_data.h
[modify] https://crrev.com/16a167f0178878c08c10ab9a1a81af6357df5e12/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
[modify] https://crrev.com/16a167f0178878c08c10ab9a1a81af6357df5e12/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/16a167f0178878c08c10ab9a1a81af6357df5e12/tools/metrics/histograms/histograms.xml

Comment 9 by dougarnett@chromium.org, Jan 11 2018

Blocking: 801275
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 12 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/704696c7bd4b8d7b2d0cf326bb06adec35845f11

commit 704696c7bd4b8d7b2d0cf326bb06adec35845f11
Author: Doug Arnett <dougarnett@chromium.org>
Date: Fri Jan 12 19:31:52 2018

Reland "Considers Cache-Control:no-transform header for committed PreviewsState"

This is a reland of 635606bad248ca3644c8039402d8debd769fdd82
Original change's description:
> Considers Cache-Control:no-transform header for committed PreviewsState
>
> Sets PreviewsState to PREVIEWS_OFF if main frame response has the
> no-transform header for an otherwise eligible client preview.
>
> Bug:  796952 
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
> Change-Id: Ic0a026c340c54529f3a8bd796599fb2aaab9e482
> Reviewed-on: https://chromium-review.googlesource.com/853122
> Reviewed-by: Steven Holte <holte@chromium.org>
> Reviewed-by: Charlie Harrison <csharrison@chromium.org>
> Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
> Commit-Queue: Doug Arnett <dougarnett@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#528500}

TBR=holte@chromium.org,csharrison@chromium.org

Bug:  796952 
Change-Id: I185f6ee9102e998e28f3bbeb4b229efeb952a10e
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/862422
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529019}
[modify] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/chrome/browser/loader/chrome_resource_dispatcher_host_delegate_unittest.cc
[modify] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/chrome/browser/previews/previews_browsertest.cc
[modify] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/chrome/browser/previews/previews_infobar_tab_helper.cc
[add] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/chrome/test/data/previews/noscript_test_with_no_transform_header.html
[add] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/chrome/test/data/previews/noscript_test_with_no_transform_header.html.mock-http-headers
[modify] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.cc
[modify] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/components/previews/content/previews_content_util.cc
[modify] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/components/previews/content/previews_content_util.h
[modify] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/components/previews/content/previews_content_util_unittest.cc
[modify] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/components/previews/core/previews_user_data.cc
[modify] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/components/previews/core/previews_user_data.h
[modify] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
[modify] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/704696c7bd4b8d7b2d0cf326bb06adec35845f11/tools/metrics/histograms/histograms.xml

Comment 11 by dougarnett@chromium.org, Jan 16 2018

Status: Fixed (was: Started)

Comment 12 by sclit...@chromium.org, Feb 28 2018

Blocking: 783372
►

Sign in to add a comment