New issue
Advanced search Search tips

Issue 721469 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocking:
issue 751427



Sign in to add a comment

Refactor access to Client LoFi signals from Blink

Project Member Reported by dougarnett@chromium.org, May 11 2017

Issue description

Currently, determining whether to perform image replacement (or report potential image replacement intervention) for the Client LoFi feature, uses specific methods that have been added to the WebFrameClient interface (ShouldUseClientLoFiForRequest and in-progress IsClientLoFiActiveForFreame) that the access implementations in the content RenderFrameImpl (which has access to the PreviewsState for the frame). This is not a scalable pattern for solving additional such needs if/when they arise.

Can we make the frame's PreviewsState available within Blink to avoid plumbing such methods? Or are there other alternatives?
 

Comment 1 by bengr@chromium.org, May 15 2017

Labels: M-61

Comment 2 by kinuko@chromium.org, May 16 2017

Thanks for filing this!
Blocking: 751427

Comment 4 by bengr@chromium.org, Nov 30 2017

Labels: -Pri-2 -M-61 Pri-3
Refreshed during triage.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 5 2018

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

commit db52c3fbcd47c7c02dfe36ef7bc5d8346e0f4a6c
Author: Doug Arnett <dougarnett@chromium.org>
Date: Fri Jan 05 17:13:13 2018

Plumbs PreviewsState into blink for use by intervention code

Replaces the specific IsClientLoFiActiveForFrame() method with a more
general GetPreviewsStateForFrame() method which may be used by other
pieces of interventions (such as deciding to not autoplay video).

Bug:  774219 , 721469 
Change-Id: I6c116ed344bb4f70f5e8116d559ec5519378eefd
Reviewed-on: https://chromium-review.googlesource.com/828146
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Scott Little <sclittle@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527304}
[modify] https://crrev.com/db52c3fbcd47c7c02dfe36ef7bc5d8346e0f4a6c/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/db52c3fbcd47c7c02dfe36ef7bc5d8346e0f4a6c/content/renderer/render_frame_impl.h
[modify] https://crrev.com/db52c3fbcd47c7c02dfe36ef7bc5d8346e0f4a6c/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/db52c3fbcd47c7c02dfe36ef7bc5d8346e0f4a6c/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/db52c3fbcd47c7c02dfe36ef7bc5d8346e0f4a6c/third_party/WebKit/Source/core/exported/LocalFrameClientImpl.cpp
[modify] https://crrev.com/db52c3fbcd47c7c02dfe36ef7bc5d8346e0f4a6c/third_party/WebKit/Source/core/exported/LocalFrameClientImpl.h
[modify] https://crrev.com/db52c3fbcd47c7c02dfe36ef7bc5d8346e0f4a6c/third_party/WebKit/Source/core/frame/LocalFrame.cpp
[modify] https://crrev.com/db52c3fbcd47c7c02dfe36ef7bc5d8346e0f4a6c/third_party/WebKit/Source/core/frame/LocalFrameClient.h
[add] https://crrev.com/db52c3fbcd47c7c02dfe36ef7bc5d8346e0f4a6c/third_party/WebKit/Source/core/frame/LocalFrameTest.cpp
[modify] https://crrev.com/db52c3fbcd47c7c02dfe36ef7bc5d8346e0f4a6c/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/db52c3fbcd47c7c02dfe36ef7bc5d8346e0f4a6c/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp
[modify] https://crrev.com/db52c3fbcd47c7c02dfe36ef7bc5d8346e0f4a6c/third_party/WebKit/public/web/WebFrameClient.h

Owner: dougarnett@chromium.org
Status: Fixed (was: Assigned)
Note: one follow-on idea was to pull WebURLRequest::PreviewsState out into its own class in blink and replace use of content::PreviewsState with it (so only one PreviewsState bitmask defn).

Sign in to add a comment