Consistently use LoFi for an entire page

content/renderer/render_frame_impl.h

Index: content/renderer/render_frame_impl.h
diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h
index ba49c52e4c9a99839cca929742f0b1f501cbc85b..4db659bc4f1570b5477661fd724d5fef70d8cf8b 100644
--- a/content/renderer/render_frame_impl.h
+++ b/content/renderer/render_frame_impl.h
@@ -18,6 +18,7 @@
 #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"
@@ -219,7 +220,7 @@ class CONTENT_EXPORT RenderFrameImpl
   // 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();
@@ -1027,6 +1028,10 @@ class CONTENT_EXPORT RenderFrameImpl
 
   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_;