Consistently use LoFi for an entire page

content/renderer/render_frame_impl.h

 // 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.
 
 #ifndef CONTENT_RENDERER_RENDER_FRAME_IMPL_H_
 #define CONTENT_RENDERER_RENDER_FRAME_IMPL_H_
 
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/files/file_path.h"
 #include "base/gtest_prod_util.h"
 #include "base/id_map.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/process/process_handle.h"
 #include "content/common/accessibility_mode_enums.h"
 #include "content/common/frame_message_enums.h"
 #include "content/common/mojo/service_registry_impl.h"
+#include "content/common/navigation_params.h"
 #include "content/public/common/console_message_level.h"
 #include "content/public/common/javascript_message_type.h"
 #include "content/public/common/referrer.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/renderer/render_frame_proxy.h"
 #include "content/renderer/renderer_webcookiejar_impl.h"
 #include "ipc/ipc_message.h"
 #include "media/blink/webmediaplayer_delegate.h"
 #include "mojo/application/public/interfaces/service_provider.mojom.h"
 #include "mojo/application/public/interfaces/shell.mojom.h"
@@ skipping 181 matching lines @@
 
   DevToolsAgent* devtools_agent() { return devtools_agent_; }
 
   // This is called right after creation with the WebLocalFrame for this
   // RenderFrame. It must be called before Initialize.
   void SetWebFrame(blink::WebLocalFrame* web_frame);
 
   // This method must be called after the frame has been added to the frame
   // tree. It creates all objects that depend on the frame being at its proper
   // spot.
-  void Initialize();
+  void Initialize(LoFiState lofi_state = LOFI_DEFAULT);

[davidben, 2015/08/27 18:53:50]

Style: no default arguments.
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Default_Argu...

[megjablon, 2015/08/27 23:11:02]

Done.

 
   // Notifications from RenderWidget.
   void WasHidden();
   void WasShown();
   void WidgetWillClose();
 
   // Start/Stop loading notifications.
   // TODO(nasko): Those are page-level methods at this time and come from
   // WebViewClient. We should move them to be WebFrameClient calls and put
   // logic in the browser side to balance starts/stops.
@@ skipping 787 matching lines @@
   RendererAccessibility* renderer_accessibility_;
 
   scoped_ptr<PermissionDispatcher> permission_client_;
 
   scoped_ptr<blink::WebAppBannerClient> app_banner_client_;
 
   scoped_ptr<blink::WebBluetooth> bluetooth_;
 
   scoped_ptr<blink::WebUSBClient> usb_client_;
 
+  // Whether or not to request a LoFi version of the resource or let the browser
+  // decide.

[davidben, 2015/08/27 18:53:50]

This is the copy of the field on the RenderFrameImpl, so "the resource" doesn't
make sense. Document exactly what this means.

(I thought RenderFrameImpl would never see LOFI_DEFAULT. It should always know
which one it is. In my sketch it was just a boolean, but perhaps I didn't
foresee some complication.)

[bengr, 2015/08/27 21:14:07]

Agreed this can/should be a bit.

[megjablon, 2015/08/27 23:11:02]

I don't think this can be a bit. We need this for the ability to turn off Lo-Fi
on mainframe requests. RenderFrameHost sends a FrameMsg_Navigate message to
RenderFrameImpl. This message includes RequestNavigationParams and for
navigations where the user wants to explicitly disable LoFi, we add LOFI_OFF to
those params. We store this in lofi_state_ in RenderFrameImpl::NavigateInternal
and in RenderFrameImpl::willSendRequest we copy the lofi_state_ to
RequestExtraData. If we only have a bit then we don't know if the mainframe
request wants to explicitly turn on or off Lo-Fi or let the browser decide.

[davidben, 2015/08/31 23:43:24]

That won't work anyway. Navigations are not synchronous. While the new
navigation is in flight and hasn't committed, the document is still live and may
make new requests.

The navigation may not even commit (perhaps a download, perhaps stopped), in
which case, the document will be around for arbitrarily long afterwards.

For navigations, the value needs to get attached to the request earlier. When we
were hashing this out, I specifically cited CreateURLRequestforNavigation.

[megjablon, 2015/09/02 21:25:42]

So you're saying we should use CreateURLRequestforNavigation instead of
NavigateInternal and immediately add the LoFiState there? How does this work
with CreateURLRequestforNavigation not getting the RequestNavigationParams.
Also, does the main frame request go through WillSendRequest after this?

[davidben, 2015/09/04 14:41:22]

I'm not super familiar with the distinctions between all the sets of parameters
anymore. You'll have to ask nasko@ about that. Presumably you'll need to add
another bit somewhere to pass lofi state down in lieu of reload-ignoring-cache,
so it can be on whichever set is appropriate.
The main frame request still goes through willSendRequest, yes, before
PlzNavigate. After PlzNavigate, the renderer process isn't involved at all and
you'll need to route this through browser-side navigation bits. (Actually,
that's another thing we'll need to fix here.)

To be clear, you *cannot* update lofi_state_ until didCommitProvisionalLoad.
Consider the following situation:

1. Global state is lofi.
2. Load a page. It is in lofi mode.
3. Global state is not lofi.
4. Tap a link on page. It loads as not lofi mode.
5. However, that load turns into a download and doesn't complete.

lofi_state_ must remain unchanged in this scenario.

[megjablon, 2015/09/09 20:54:16]

@nasko can you take a look?

+  LoFiState lofi_state_;
+
 #if defined(ENABLE_WEBVR)
   // The VR dispatcher attached to the frame, lazily initialized.
   scoped_ptr<VRDispatcher> vr_dispatcher_;
 #endif
 
 #if defined(OS_MACOSX) || defined(OS_ANDROID)
   // The external popup for the currently showing select popup.
   scoped_ptr<ExternalPopupMenu> external_popup_menu_;
 #endif
 
   base::WeakPtrFactory<RenderFrameImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderFrameImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_RENDERER_RENDER_FRAME_IMPL_H_