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

content/renderer/render_frame_impl_browsertest.cc

 // Copyright 2015 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 <stdint.h>
 
 #include "base/command_line.h"
 #include "base/debug/leak_annotations.h"
 #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 {
 const int32_t kSubframeRouteId = 20;
 const int32_t kSubframeWidgetRouteId = 21;
 const int32_t kFrameProxyRouteId = 22;
 }  // namespace
 
 namespace content {
 
 // RenderFrameImplTest creates a RenderFrameImpl that is a child of the
 // main frame, and has its own RenderWidget. This behaves like an out
 // of process frame even though it is in the same process as its parent.
 class RenderFrameImplTest : public RenderViewTest {
  public:
   ~RenderFrameImplTest() override {}
 
   void SetUp() override {
     RenderViewTest::SetUp();
     EXPECT_TRUE(static_cast<RenderFrameImpl*>(view_->GetMainRenderFrame())

[Charlie Reis, 2016/02/04 19:30:30]

nit: Let's use the new helper here.

[megjablon, 2016/02/04 20:02:31]

Done.

                     ->is_main_frame_);
 
     FrameMsg_NewFrame_WidgetParams widget_params;
     widget_params.routing_id = kSubframeWidgetRouteId;
     widget_params.hidden = false;
 
     IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess());
 
     LoadHTML("Parent frame <iframe name='frame'></iframe>");
 
@@ skipping 13 matching lines @@
 
   void TearDown() override {
 #if defined(LEAK_SANITIZER)
      // Do this before shutting down V8 in RenderViewTest::TearDown().
      // http://crbug.com/328552
      __lsan_do_leak_check();
 #endif
      RenderViewTest::TearDown();
   }
 
+  void SetIsUsingLoFi(RenderFrameImpl* frame, bool is_using_lofi) {
+    frame->is_using_lofi_ = is_using_lofi;
+  }
+
+  RenderFrameImpl* main_frame() {

[Charlie Reis, 2016/02/04 19:30:30]

Style nit: GetMainRenderFrame()

(hacker_case is for trivial methods that can generally be inlined, but this
calls a virtual method that already CamelCase.)

[megjablon, 2016/02/04 20:02:31]

Done.

+    return static_cast<RenderFrameImpl*>(view_->GetMainRenderFrame());
+  }
+
   RenderFrameImpl* frame() { return frame_; }
 
   content::RenderWidget* frame_widget() const {
     return frame_->render_widget_.get();
   }
 
  private:
   RenderFrameImpl* frame_;
   FakeCompositorDependencies compositor_deps_;
 };
@@ skipping 86 matching lines @@
   frame_widget()->OnMessageReceived(close_message);
 
   ViewMsg_WasShown was_shown_message(0, true, ui::LatencyInfo());
   static_cast<RenderViewImpl*>(view_)->OnMessageReceived(was_shown_message);
 
   // This test is primarily checking that this case does not crash, but
   // observers should still be notified.
   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(main_frame(), true);
+  SetIsUsingLoFi(frame(), true);
+  EXPECT_TRUE(main_frame()->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());
+  main_frame()->didNavigateWithinPage(main_frame()->GetWebFrame(), item,
+                                      blink::WebStandardCommit);
+  EXPECT_TRUE(main_frame()->IsUsingLoFi());

[Charlie Reis, 2016/02/04 19:30:30]

Can you add a test for the subframe doing a didCommitProvisionalLoad, ensuring
that it stays in LoFi mode?

[megjablon, 2016/02/04 20:02:31]

Done.

+
+  // The main frame's LoFi state should be reset to off on commit.
+  DocumentState* document_state =
+      DocumentState::FromDataSource(main_frame()->GetWebFrame()->dataSource());
+  static_cast<NavigationStateImpl*>(document_state->navigation_state())
+      ->set_was_within_same_page(false);
+
+  main_frame()->didCommitProvisionalLoad(main_frame()->GetWebFrame(), item,

[Charlie Reis, 2016/02/04 19:30:30]

It's a little surprising to me that these work with blank HistoryItems, but
that's fine.

It's also a little unexpected that this test deletes the subframe, since there
are pointers to it in the test class above.  I suppose it's safe for now since
we don't try to access it in TearDown, but maybe it's worth a comment saying
that the subframe is destroyed after this.

[megjablon, 2016/02/04 20:02:31]

Done. Comment added as discussed offline.

+                                         blink::WebStandardCommit);
+  EXPECT_FALSE(main_frame()->IsUsingLoFi());
+}
+
 }  // namespace