Reload Lo-Fi images inline instead of reloading the whole page

content/renderer/render_frame_impl_browsertest.cc

Index: content/renderer/render_frame_impl_browsertest.cc
diff --git a/content/renderer/render_frame_impl_browsertest.cc b/content/renderer/render_frame_impl_browsertest.cc
index 311a4556e6ca49db85d3b38bdc8c48d56e4e817b..6c8b0433327a39ef22933fc10d7c8ee0d5883aab 100644
--- a/content/renderer/render_frame_impl_browsertest.cc
+++ b/content/renderer/render_frame_impl_browsertest.cc
@@ -9,15 +9,18 @@
 #include "build/build_config.h"
 #include "content/common/frame_messages.h"
 #include "content/common/view_messages.h"
+#include "content/public/renderer/document_state.h"
 #include "content/public/test/frame_load_waiter.h"
 #include "content/public/test/render_view_test.h"
 #include "content/public/test/test_utils.h"
+#include "content/renderer/navigation_state_impl.h"
 #include "content/renderer/render_frame_impl.h"
 #include "content/renderer/render_view_impl.h"
 #include "content/test/fake_compositor_dependencies.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/WebKit/public/platform/WebURLRequest.h"
 #include "third_party/WebKit/public/web/WebFrameOwnerProperties.h"
+#include "third_party/WebKit/public/web/WebHistoryItem.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
 
 namespace {
@@ -37,8 +40,7 @@ class RenderFrameImplTest : public RenderViewTest {
 
   void SetUp() override {
     RenderViewTest::SetUp();
-    EXPECT_TRUE(static_cast<RenderFrameImpl*>(view_->GetMainRenderFrame())
-                    ->is_main_frame_);
+    EXPECT_TRUE(GetMainRenderFrame()->is_main_frame_);
 
     FrameMsg_NewFrame_WidgetParams widget_params;
     widget_params.routing_id = kSubframeWidgetRouteId;
@@ -71,6 +73,14 @@ class RenderFrameImplTest : public RenderViewTest {
      RenderViewTest::TearDown();
   }
 
+  void SetIsUsingLoFi(RenderFrameImpl* frame, bool is_using_lofi) {
+    frame->is_using_lofi_ = is_using_lofi;
+  }
+
+  RenderFrameImpl* GetMainRenderFrame() {
+    return static_cast<RenderFrameImpl*>(view_->GetMainRenderFrame());
+  }
+
   RenderFrameImpl* frame() { return frame_; }
 
   content::RenderWidget* frame_widget() const {
@@ -177,4 +187,48 @@ TEST_F(RenderFrameImplTest, MAYBE_FrameWasShownAfterWidgetClose) {
   EXPECT_TRUE(observer.visible());
 }
 
+// Test that LoFi state only updates for new main frame documents. Subframes
+// inherit from the main frame and should not change at commit time.
+TEST_F(RenderFrameImplTest, LoFiNotUpdatedOnSubframeCommits) {
+  SetIsUsingLoFi(GetMainRenderFrame(), true);
+  SetIsUsingLoFi(frame(), true);
+  EXPECT_TRUE(GetMainRenderFrame()->IsUsingLoFi());
+  EXPECT_TRUE(frame()->IsUsingLoFi());
+
+  blink::WebHistoryItem item;
+  item.initialize();
+
+  // The main frame's and subframe's LoFi states should stay the same on
+  // navigations within the page.
+  frame()->didNavigateWithinPage(frame()->GetWebFrame(), item,
+                                 blink::WebStandardCommit);
+  EXPECT_TRUE(frame()->IsUsingLoFi());
+  GetMainRenderFrame()->didNavigateWithinPage(
+      GetMainRenderFrame()->GetWebFrame(), item, blink::WebStandardCommit);
+  EXPECT_TRUE(GetMainRenderFrame()->IsUsingLoFi());
+
+  // The subframe's LoFi state should not be reset on commit.
+  DocumentState* document_state =
+      DocumentState::FromDataSource(frame()->GetWebFrame()->dataSource());
+  static_cast<NavigationStateImpl*>(document_state->navigation_state())
+      ->set_was_within_same_page(false);
+
+  frame()->didCommitProvisionalLoad(frame()->GetWebFrame(), item,
+                                    blink::WebStandardCommit);
+  EXPECT_TRUE(frame()->IsUsingLoFi());
+
+  // The main frame's LoFi state should be reset to off on commit.
+  document_state = DocumentState::FromDataSource(
+      GetMainRenderFrame()->GetWebFrame()->dataSource());
+  static_cast<NavigationStateImpl*>(document_state->navigation_state())
+      ->set_was_within_same_page(false);
+
+  // Calling didCommitProvisionalLoad is not representative of a full navigation
+  // but serves the purpose of testing the LoFi state logic.
+  GetMainRenderFrame()->didCommitProvisionalLoad(
+      GetMainRenderFrame()->GetWebFrame(), item, blink::WebStandardCommit);
+  EXPECT_FALSE(GetMainRenderFrame()->IsUsingLoFi());
+  EXPECT_TRUE(frame()->IsUsingLoFi());

[Charlie Reis, 2016/02/04 20:08:35]

This last line looks like the wrong behavior at first glance, and it's kind of
enforcing the incomplete navigation behavior (which isn't useful for the code
and might be a hurdle if someone wants to turn these into real navigations
later).

I'd suggest leaving this last EXPECT out and replacing with:

// The subframe would be deleted here after a cross-document navigation. It
happens to be left around in this test because this does not simulate the frame
detach.

[megjablon, 2016/02/04 20:48:09]

Done.

+}
+
 }  // namespace