PlzNavigate: Fix wrong "Cache-Control" header.

third_party/WebKit/Source/web/WebLocalFrameImpl.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 1681 matching lines @@
   if (!webframe_child)
     return nullptr;
 
   webframe_child->InitializeCoreFrame(*GetFrame()->GetPage(), owner_element,
                                       name);
   // Initializing the core frame may cause the new child to be detached, since
   // it may dispatch a load event in the parent.
   if (!webframe_child->Parent())
     return nullptr;
 
-  // If we're moving in the back/forward list, we might want to replace the
-  // content of this child frame with whatever was there at that point.
+  FrameLoadRequest new_request = request;
+  FrameLoadType child_load_type = kFrameLoadTypeStandard;
   HistoryItem* child_item = nullptr;
-  if (IsBackForwardLoadType(
-          GetFrame()->Loader().GetDocumentLoader()->LoadType()) &&
-      !GetFrame()->GetDocument()->LoadEventFinished())
-    child_item = webframe_child->Client()->HistoryItemForNewChildFrame();
 
-  FrameLoadRequest new_request = request;
-  FrameLoadType load_type = kFrameLoadTypeStandard;
-  if (child_item) {
-    new_request = FrameLoadRequest(
-        request.OriginDocument(), child_item->GenerateResourceRequest(
-                                      WebCachePolicy::kUseProtocolCachePolicy));
-    load_type = kFrameLoadTypeInitialHistoryLoad;
+  if (!GetFrame()->GetDocument()->LoadEventFinished()) {
+    FrameLoadType load_type =
+        GetFrame()->Loader().GetDocumentLoader()->LoadType();
+    switch (load_type) {
+      case kFrameLoadTypeStandard:
+      case kFrameLoadTypeReplaceCurrentItem:
+      case kFrameLoadTypeInitialInChildFrame:
+        break;
+
+      // If we're moving in the back/forward list, we might want to replace the

[Takashi Toyoshima, 2017/05/09 10:57:25]

Modified code for bypassing cache look good to me, but may I ask a question to
understand existing code here?

Whey here we could not prefer cached content for history navigation as we are
doing in the legacy code path?

Also, is it fine that we have minor logic differences between this code path and
DetermineWebCachePolicy in FrameFetchContext.cpp? E.g., not checking http
method, not checking conditional request.

[arthursonzogni, 2017/05/09 12:49:06]

I will let other peoples answer the questions because I am not really used to
this code.

FYI: With PlzNavigate, we are no more using the ResourceFetcher for loading the
frame. Thus the FrameFetchContext is not used for determining the right cache
flags. That's the main reason why it was not working before this patch.
Do you know what prevents us to determine the right FrameLoadType and the right
CachePolicy in the FrameLoader?

[Charlie Reis, 2017/05/10 05:56:40]

Sorry, I don't follow any of these questions.  This CL doesn't appear to change
the back/forward cache behavior, which seems good to me.  (We use
kUseProtocolCachePolicy for kFrameLoadTypeBackForward and
kFrameLoadTypeInitialHistoryLoad both before and after the CL.)  If there's a
reason to change that, it should probably be done separately from this CL.

+      // content of this child frame with whatever was there at that point.
+      case kFrameLoadTypeBackForward:
+      case kFrameLoadTypeInitialHistoryLoad:
+        child_item = webframe_child->Client()->HistoryItemForNewChildFrame();
+        if (child_item) {
+          child_load_type = kFrameLoadTypeInitialHistoryLoad;
+          new_request =
+              FrameLoadRequest(request.OriginDocument(),
+                               child_item->GenerateResourceRequest(
+                                   WebCachePolicy::kUseProtocolCachePolicy));
+        }
+        break;
+
+      // If we're in a middle of a reload. The FrameLoadType is propagated to

[Charlie Reis, 2017/05/10 05:56:40]

nit: Drop "If"

[arthursonzogni, 2017/05/10 08:05:13]

Done.

+      // its children only if it is a ReloadByPassingCache, else it becomes a

[Charlie Reis, 2017/05/10 05:56:40]

nit: Don't capitalize P.

[arthursonzogni, 2017/05/10 08:05:13]

Done.

+      // standard load.
+      case kFrameLoadTypeReload:
+        break;
+      case kFrameLoadTypeReloadBypassingCache:
+        child_load_type = kFrameLoadTypeReloadBypassingCache;
+        new_request.GetResourceRequest().SetCachePolicy(
+            WebCachePolicy::kBypassingCache);
+        break;
+    }
   }
-  webframe_child->GetFrame()->Loader().Load(new_request, load_type, child_item);
+
+  webframe_child->GetFrame()->Loader().Load(new_request, child_load_type,
+                                            child_item);
 
   // Note a synchronous navigation (about:blank) would have already processed
   // onload, so it is possible for the child frame to have already been
   // detached by script in the page.
   if (!webframe_child->Parent())
     return nullptr;
   return webframe_child->GetFrame();
 }
 
 void WebLocalFrameImpl::DidChangeContentsSize(const IntSize& size) {
@@ skipping 844 matching lines @@
 TextCheckerClient& WebLocalFrameImpl::GetTextCheckerClient() const {
   return *text_checker_client_;
 }
 
 void WebLocalFrameImpl::SetTextCheckClient(
     WebTextCheckClient* text_check_client) {
   text_check_client_ = text_check_client;
 }
 
 }  // namespace blink