Consistently use LoFi for an entire page

content/renderer/render_frame_impl.cc

 // Copyright 2013 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.
 
 #include "content/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 
 #include "base/auto_reset.h"
@@ skipping 725 matching lines @@
 #endif
       has_played_media_(false),
       devtools_agent_(nullptr),
       geolocation_dispatcher_(NULL),
       push_messaging_dispatcher_(NULL),
       presentation_dispatcher_(NULL),
       screen_orientation_dispatcher_(NULL),
       manifest_manager_(NULL),
       accessibility_mode_(AccessibilityModeOff),
       renderer_accessibility_(NULL),
+      lofi_state_(LOFI_DEFAULT),
       weak_factory_(this) {
   std::pair<RoutingIDFrameMap::iterator, bool> result =
       g_routing_id_frame_map.Get().insert(std::make_pair(routing_id_, this));
   CHECK(result.second) << "Inserting a duplicate item.";
 
   RenderThread::Get()->AddRoute(routing_id_, this);
 
   render_view_->RegisterRenderFrame(this);
 
   // Everything below subclasses RenderFrameObserver and is automatically
@@ skipping 49 matching lines @@
 void RenderFrameImpl::SetWebFrame(blink::WebLocalFrame* web_frame) {
   DCHECK(!frame_);
 
   std::pair<FrameMap::iterator, bool> result = g_frame_map.Get().insert(
       std::make_pair(web_frame, this));
   CHECK(result.second) << "Inserting a duplicate item.";
 
   frame_ = web_frame;
 }
 
-void RenderFrameImpl::Initialize() {
+void RenderFrameImpl::Initialize(LoFiState lofi_state) {
   is_subframe_ = !!frame_->parent();
   is_local_root_ = !frame_->parent() || frame_->parent()->isWebRemoteFrame();
+  lofi_state_ = lofi_state;
 
 #if defined(ENABLE_PLUGINS)
   new PepperBrowserConnection(this);
 #endif
   new SharedWorkerRepository(this);
 
   if (is_local_root_ && !render_frame_proxy_) {
     // DevToolsAgent is a RenderFrameObserver, and will destruct itself
     // when |this| is deleted.
     devtools_agent_ = new DevToolsAgent(this);
@@ skipping 1177 matching lines @@
       ServiceRegistryJsWrapper::kModuleName,
       ServiceRegistryJsWrapper::Create(isolate, &service_registry_).ToV8());
 }
 
 void RenderFrameImpl::AddMessageToConsole(ConsoleMessageLevel level,
                                           const std::string& message) {
   if (devtools_agent_)
     devtools_agent_->AddMessageToConsole(level, message);
 }
 
+bool RenderFrameImpl::IsLoFiOn() {
+  return lofi_state_ == LOFI_ON;
+}
+
 // blink::WebFrameClient implementation ----------------------------------------
 
 blink::WebPlugin* RenderFrameImpl::createPlugin(
     blink::WebLocalFrame* frame,
     const blink::WebPluginParams& params) {
   DCHECK_EQ(frame_, frame);
   blink::WebPlugin* plugin = NULL;
   if (GetContentClient()->renderer()->OverrideCreatePlugin(
           this, frame, params, &plugin)) {
     return plugin;
@@ skipping 199 matching lines @@
 
   // Create the RenderFrame and WebLocalFrame, linking the two.
   RenderFrameImpl* child_render_frame = RenderFrameImpl::Create(
       render_view_.get(), child_routing_id);
   blink::WebLocalFrame* web_frame =
       WebLocalFrame::create(scope, child_render_frame);
   child_render_frame->SetWebFrame(web_frame);
 
   // Add the frame to the frame tree and initialize it.
   parent->appendChild(web_frame);
-  child_render_frame->Initialize();
+  child_render_frame->Initialize(lofi_state_);
 
   return web_frame;
 }
 
 void RenderFrameImpl::didChangeOpener(blink::WebFrame* opener) {
   // Only active frames are able to disown their opener.
   if (!opener && is_swapped_out_)
     return;
 
   // Only a local frame should be able to update another frame's opener.
@@ skipping 451 matching lines @@
     const blink::WebHistoryItem& item,
     blink::WebHistoryCommitType commit_type) {
   TRACE_EVENT2("navigation", "RenderFrameImpl::didCommitProvisionalLoad",
                "id", routing_id_,
                "url", GetLoadingUrl().possibly_invalid_spec());
   DCHECK(!frame_ || frame_ == frame);
   DocumentState* document_state =
       DocumentState::FromDataSource(frame->dataSource());
   NavigationStateImpl* navigation_state =
       static_cast<NavigationStateImpl*>(document_state->navigation_state());
+  WebURLResponseExtraDataImpl* extra_data = GetExtraDataFromResponse(
+      frame->dataSource()->response());
+  lofi_state_ = extra_data && extra_data->is_lofi() ? LOFI_ON : LOFI_OFF;
 
   if (proxy_routing_id_ != MSG_ROUTING_NONE) {
     RenderFrameProxy* proxy =
         RenderFrameProxy::FromRoutingID(proxy_routing_id_);
     CHECK(proxy);
     proxy->web_frame()->swap(frame_);
     proxy_routing_id_ = MSG_ROUTING_NONE;
 
     // If this is the main frame going from a remote frame to a local frame,
     // it needs to set RenderViewImpl's pointer for the main frame to itself
@@ skipping 634 matching lines @@
   extra_data->set_allow_download(
       navigation_state->common_params().allow_download);
   extra_data->set_transition_type(transition_type);
   extra_data->set_should_replace_current_entry(should_replace_current_entry);
   extra_data->set_transferred_request_child_id(
       navigation_state->start_params().transferred_request_child_id);
   extra_data->set_transferred_request_request_id(
       navigation_state->start_params().transferred_request_request_id);
   extra_data->set_service_worker_provider_id(provider_id);
   extra_data->set_stream_override(stream_override.Pass());
+  // TODO(megjablon): Set the navigation params for single image loads to
+  // LOFI_OFF and remove the dependency on ReloadBypassingCache.
+  if (request.cachePolicy() == WebURLRequest::ReloadBypassingCache)
+    extra_data->set_lofi_state(LOFI_OFF);
+  else
+    extra_data->set_lofi_state(lofi_state_);
   request.setExtraData(extra_data);
 
   // TODO(creis): Update prefetching to work with out-of-process iframes.
   WebFrame* top_frame = frame->top();
   if (top_frame && top_frame->isWebLocalFrame()) {
     DocumentState* top_document_state =
         DocumentState::FromDataSource(top_frame->dataSource());
     if (top_document_state) {
       // TODO(gavinp): separate out prefetching and prerender field trials
       // if the rel=prerender rel type is sticking around.
@@ skipping 1106 matching lines @@
           switches::kEnableBrowserSideNavigation);
   bool is_reload = IsReload(common_params.navigation_type);
   bool is_history_navigation = request_params.page_state.IsValid();
   WebURLRequest::CachePolicy cache_policy =
       WebURLRequest::UseProtocolCachePolicy;
   RenderFrameImpl::PrepareRenderViewForNavigation(
       common_params.url, request_params, &is_reload, &cache_policy);
 
   GetContentClient()->SetActiveURL(common_params.url);
 
+  lofi_state_ = request_params.lofi_state;

[davidben, 2015/09/21 17:58:15]

This still hasn't been resolved. lofi_state_ must only be updated on commit, not
when you start a navigation. See previous discussions on whether this field was
a LofiState or a bool.

[davidben, 2015/09/21 18:09:01]

To clarify, this isn't a "layering; we'll fix this later" problem. It's a
correctness problem. Until the navigation has committed, it's not correct to
change any state relating to the current navigation. The state needs to flow at
commit from the ResourceResponseInfo sent in ResourceMsg_ReceivedResponse to
didCommitProvisionalLoad and be set at that point and only at that point. That
bit then gets stapled to subresources at willSendRequest. Frame requests will
hit a slightly different codepath.

The key thing to always keep in mind is that navigations (frame requests) are
completely different from subresources. It is expected that you will need to
treat them different for this sort of thing. (Treating them the same is usually
a red flag that something is wrong.)

(In fact, I'm not sure this CL works at all for renderer-initiated navigations.
Not every navigation hits this function.)

[megjablon, 2015/09/22 20:35:52]

For renderer initiated navigations, we don't need to set Lo-Fi state because it
will always be LOFI_DEFAULT, and we have the NQE decide. This is used by browser
initiated navigations to force Lo-Fi on or off and will be used by the "Load
images" snackbar (to be hooked up in a follow-up cl). Do you have any
suggestions for a path to force the navigation to take the value of
RequestNavigationParams that doesn't break this correctness? Maybe the problem
is we're combining the navigation signal to force Lo-Fi on or off with the
response value at commit that sets the state. These values would be the same for
the navigations with forced Lo-Fi state, and that's why it was being set early
here.

[davidben, 2015/09/22 22:20:50]

I think we were talking about different things. Though I see that
didCommitProvisionalLoad *also* sets lofi_state_. Sorry, I missed that somehow.
The part I was thinking of works fine for renderer-initiated navigations. All
you actually want to do is ditch this line, switch it to a boolean, and then
plumb the value for navigations differently.
Why not attach it to the extraData in CreateURLRequestForNavigation? You'll just
need to make tweak the willSendRequest code to only inherit on subresource
requests, but not navigations, since those are pretty different. That would
translate to PlzNavigate well since you get a browser-side
CreateURLRequestForNavigation-looking function, but not willSendRequest in the
renderer because the renderer does not exist.
Yes! That's it in a nutshell. Thanks for phrasing it much better than I did. :-)
They're not quite the same because a navigation may not commit, and the previous
document may still issue subresource requests before this commit point.

[megjablon, 2015/09/23 01:39:04]

This sounds good. I'm working on implementing this. This may be a simple
question, but in willSendRequest how do I check if the request is a subresource?

[megjablon, 2015/09/23 23:15:24]

I took a stab at this. PTAL!

+
   // If this frame isn't in the same process as the main frame, it may naively
   // assume that this is the first navigation in the iframe, but this may not
   // actually be the case. Inform the frame's state machine if this frame has
   // already committed other loads.
   if (request_params.has_committed_real_load && frame_->parent())
     frame_->setCommittedFirstRealLoad();
 
   if (is_reload && !render_view_->history_controller()->GetCurrentEntry()) {
     // We cannot reload if we do not have any history state.  This happens, for
     // example, when recovering from a crash.
@@ skipping 616 matching lines @@
   mojo::ServiceProviderPtr service_provider;
   mojo::URLRequestPtr request(mojo::URLRequest::New());
   request->url = mojo::String::From(url);
   mojo_shell_->ConnectToApplication(request.Pass(), GetProxy(&service_provider),
                                     nullptr, nullptr,
                                     base::Bind(&OnGotContentHandlerID));
   return service_provider.Pass();
 }
 
 }  // namespace content