Refactor FrameLoader loading interface

Source/core/loader/FrameLoader.h

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) Research In Motion Limited 2009. All rights reserved.
  * Copyright (C) 2011 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:
  *
@@ skipping 48 matching lines @@
 class SubstituteData;
 
 struct FrameLoadRequest;
 
 CORE_EXPORT bool isBackForwardLoadType(FrameLoadType);
 
 class CORE_EXPORT FrameLoader final {
     WTF_MAKE_NONCOPYABLE(FrameLoader);
     DISALLOW_ALLOCATION();
 public:
-    static ResourceRequest requestFromHistoryItem(HistoryItem*, ResourceRequestCachePolicy);
+    static ResourceRequest resourceRequestFromHistoryItem(HistoryItem*, ResourceRequestCachePolicy);
+    static FrameLoadRequest frameRequestForReload(
+        const ResourceRequest&, ClientRedirectPolicy = NotClientRedirect);
 
     FrameLoader(LocalFrame*);
     ~FrameLoader();
 
     void init();
 
+    ResourceRequest resourceRequestForReload(const LocalFrame&, FrameLoadType,

[Nate Chapin, 2015/06/02 17:19:35]

No need to take the LocalFrame& now, we can just use m_frame?

[clamy, 2015/06/03 14:39:14]

Done.

+        const KURL& overrideURL = KURL(), ClientRedirectPolicy = NotClientRedirect);
+
     ProgressTracker& progress() const { return *m_progressTracker; }
 
-    // These functions start a load. All eventually call into startLoad() or loadInSameDocument().
-    void load(const FrameLoadRequest&); // The entry point for non-reload, non-history loads.
-    void reload(ReloadPolicy, const KURL& overrideURL = KURL(), ClientRedirectPolicy = NotClientRedirect);
-    void loadHistoryItem(HistoryItem*, FrameLoadType = FrameLoadTypeBackForward,
-        HistoryLoadType = HistoryDifferentDocumentLoad,
-        ResourceRequestCachePolicy = UseProtocolCachePolicy); // The entry point for all back/forward loads
+    // Starts a load. It will eventually call startLoad() or
+    // loadInSameDocument(). For history navigations or reloads, an appropriate
+    // FrameLoadType should be given. Otherwise, FrameLoadTypeStandard should be
+    // used (and the final FrameLoadType will be computed). For history
+    // navigations, a history item and a HistoryLoadType should also be provided.
+    void load(const FrameLoadRequest&, FrameLoadType = FrameLoadTypeStandard,
+        HistoryItem* = nullptr, HistoryLoadType = HistoryDifferentDocumentLoad);
 
     static void reportLocalLoadFailed(LocalFrame*, const String& url);
 
     // Warning: stopAllLoaders can and will detach the LocalFrame out from under you. All callers need to either protect the LocalFrame
     // or guarantee they won't in any way access the LocalFrame after stopAllLoaders returns.
     void stopAllLoaders();
 
     // FIXME: clear() is trying to do too many things. We should break it down into smaller functions.
     void clear();
 
@@ skipping 123 matching lines @@
     RefPtr<DocumentLoader> m_documentLoader;
     RefPtr<DocumentLoader> m_provisionalDocumentLoader;
     RefPtr<DocumentLoader> m_policyDocumentLoader;
 
     RefPtrWillBeMember<HistoryItem> m_currentItem;
     RefPtrWillBeMember<HistoryItem> m_provisionalItem;
 
     struct DeferredHistoryLoad {
         DISALLOW_ALLOCATION();
     public:
-        DeferredHistoryLoad(HistoryItem* item, HistoryLoadType type, ResourceRequestCachePolicy cachePolicy)
-            : m_item(item)
-            , m_type(type)
-            , m_cachePolicy(cachePolicy)
+        DeferredHistoryLoad(ResourceRequest request, HistoryItem* item, FrameLoadType loadType,
+            HistoryLoadType historyLoadType)
+            : m_request(request)
+            , m_item(item)
+            , m_loadType(loadType)
+            , m_historyLoadType(historyLoadType)
+            , m_isValid(true)
         {
         }
 
-        DeferredHistoryLoad() { }
-
-        bool isValid() { return m_item; }
+        DeferredHistoryLoad() : m_isValid(false) { }
 
         DEFINE_INLINE_TRACE()
         {
-            visitor->trace(m_item);

[dcheng, 2015/06/02 18:42:02]

Why was this removed? This probably breaks Oilpan.

[clamy, 2015/06/03 14:39:14]

Done.

         }
 
+        bool isValid() { return m_isValid; }
+
+        ResourceRequest m_request;
         RefPtrWillBeMember<HistoryItem> m_item;
-        HistoryLoadType m_type;
-        ResourceRequestCachePolicy m_cachePolicy;
+        FrameLoadType m_loadType;
+        HistoryLoadType m_historyLoadType;
+        bool m_isValid;

[dcheng, 2015/06/02 18:42:02]

Rather than having a m_isValid member, I'd rather see m_deferredHistoryLoad
wrapped in an OwnPtr<>.

[clamy, 2015/06/03 14:39:14]

Done.

     };
 
     DeferredHistoryLoad m_deferredHistoryLoad;
 
     bool m_inStopAllLoaders;
 
     Timer<FrameLoader> m_checkTimer;
 
     bool m_didAccessInitialDocument;
     Timer<FrameLoader> m_didAccessInitialDocumentTimer;
 
     SandboxFlags m_forcedSandboxFlags;
 };
 
 } // namespace blink
 
 #endif // FrameLoader_h