NavigationAction has too many constructors
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Jun 2017 17:21:47 +0000 (17:21 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Jun 2017 17:21:47 +0000 (17:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173484

Reviewed by Brady Eidson.

A NavigationAction object is an immutable object that represents the details of a
navigation, including the type of a navigation (e.g. link click), what triggered
the navigation, and the external URL policy to use for the navigation. Over time
the number of NavigationAction constructor overloads (not including copy/move
constructors) has grown to 12 to support different combinations of details.
We can use default values to reduce the number of constructors to 2 (not including
copy/move constructors).

No behavior changed. So, no new tests.

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::continueLoadAfterNewWindowPolicy): Pass NavigationType::Other when
instantiating NavigationAction.
(WebCore::FrameLoader::loadDifferentDocumentItem): Fix order of arguments now that
the constructor overload that takes a NavigationType takes the Event* as the fourth argument,
not the third. Also, use C++11 brace initialization syntax when instantiating a NavigationAction.
(WebCore::createWindow):
* loader/NavigationAction.cpp: Remove unnecessary #include of header ScriptController.h.
Include header Event.h.
(WebCore::NavigationAction::NavigationAction):
* loader/NavigationAction.h: Forward declare class Event and remove #include of header Event.h.
Make copy constructor, copy assignment operator, move constructor, and move assignment operator
out-of-line to avoid the need to include header Event.h. Export the copy constructor so that it
can be used from WebKit on the Apple Windows port. Move ShouldOpenExternalURLsPolicy to be after
NavigationType to reduce the size of the class by 8 bytes.
(WebCore::NavigationAction::NavigationAction):
* loader/PolicyChecker.cpp: Include header Event.h.
* page/Performance.cpp: Ditto.
* replay/ReplayController.cpp: Ditto.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@218599 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/NavigationAction.cpp
Source/WebCore/loader/NavigationAction.h
Source/WebCore/loader/PolicyChecker.cpp
Source/WebCore/page/Performance.cpp
Source/WebCore/replay/ReplayController.cpp

index 10a4844..ddf7b05 100644 (file)
@@ -1,3 +1,40 @@
+2017-06-20  Daniel Bates  <dabates@apple.com>
+
+        NavigationAction has too many constructors
+        https://bugs.webkit.org/show_bug.cgi?id=173484
+
+        Reviewed by Brady Eidson.
+
+        A NavigationAction object is an immutable object that represents the details of a
+        navigation, including the type of a navigation (e.g. link click), what triggered
+        the navigation, and the external URL policy to use for the navigation. Over time
+        the number of NavigationAction constructor overloads (not including copy/move
+        constructors) has grown to 12 to support different combinations of details.
+        We can use default values to reduce the number of constructors to 2 (not including
+        copy/move constructors).
+
+        No behavior changed. So, no new tests.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::continueLoadAfterNewWindowPolicy): Pass NavigationType::Other when
+        instantiating NavigationAction.
+        (WebCore::FrameLoader::loadDifferentDocumentItem): Fix order of arguments now that
+        the constructor overload that takes a NavigationType takes the Event* as the fourth argument,
+        not the third. Also, use C++11 brace initialization syntax when instantiating a NavigationAction.
+        (WebCore::createWindow):
+        * loader/NavigationAction.cpp: Remove unnecessary #include of header ScriptController.h.
+        Include header Event.h.
+        (WebCore::NavigationAction::NavigationAction):
+        * loader/NavigationAction.h: Forward declare class Event and remove #include of header Event.h.
+        Make copy constructor, copy assignment operator, move constructor, and move assignment operator
+        out-of-line to avoid the need to include header Event.h. Export the copy constructor so that it
+        can be used from WebKit on the Apple Windows port. Move ShouldOpenExternalURLsPolicy to be after
+        NavigationType to reduce the size of the class by 8 bytes.
+        (WebCore::NavigationAction::NavigationAction):
+        * loader/PolicyChecker.cpp: Include header Event.h.
+        * page/Performance.cpp: Ditto.
+        * replay/ReplayController.cpp: Ditto.
+
 2017-06-20  Konstantin Tokarev  <annulen@yandex.ru>
 
         Rename OrientationNotifer.h to OrientationNotifier.h
index aa4b808..958380b 100644 (file)
@@ -3174,7 +3174,7 @@ void FrameLoader::continueLoadAfterNewWindowPolicy(const ResourceRequest& reques
         mainFrame->document()->setReferrerPolicy(frame->document()->referrerPolicy());
     }
 
-    NavigationAction newAction(request, action.shouldOpenExternalURLsPolicy());
+    NavigationAction newAction { request, NavigationType::Other, action.shouldOpenExternalURLsPolicy() };
     mainFrame->loader().loadWithNavigationAction(request, newAction, LockHistory::No, FrameLoadType::Standard, formState, allowNavigationToInvalidURL);
 }
 
@@ -3411,7 +3411,7 @@ void FrameLoader::loadDifferentDocumentItem(HistoryItem& item, FrameLoadType loa
             action = NavigationAction(request, loadType, isFormSubmission, event, shouldOpenExternalURLsPolicy);
         } else {
             request.setCachePolicy(ReturnCacheDataElseLoad);
-            action = NavigationAction(request, NavigationType::FormResubmitted, event, shouldOpenExternalURLsPolicy);
+            action = NavigationAction(request, NavigationType::FormResubmitted, shouldOpenExternalURLsPolicy, event);
         }
     } else {
         switch (loadType) {
@@ -3700,7 +3700,7 @@ RefPtr<Frame> createWindow(Frame& openerFrame, Frame& lookupFrame, const FrameLo
         return nullptr;
 
     ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy = shouldOpenExternalURLsPolicyToApply(openerFrame, request.shouldOpenExternalURLsPolicy());
-    Page* page = oldPage->chrome().createWindow(openerFrame, requestWithReferrer, features, NavigationAction(requestWithReferrer.resourceRequest(), shouldOpenExternalURLsPolicy));
+    Page* page = oldPage->chrome().createWindow(openerFrame, requestWithReferrer, features, NavigationAction(requestWithReferrer.resourceRequest(), NavigationType::Other, shouldOpenExternalURLsPolicy));
     if (!page)
         return nullptr;
 
index 3624b42..895c085 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2016 Apple Inc.  All rights reserved.
+ * Copyright (C) 2006-2017 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 
 #include "Event.h"
 #include "FrameLoader.h"
-#include "ScriptController.h"
 
 namespace WebCore {
 
+NavigationAction::~NavigationAction() = default;
+
+NavigationAction::NavigationAction(const NavigationAction&) = default;
+NavigationAction::NavigationAction(NavigationAction&&) = default;
+
+NavigationAction& NavigationAction::operator=(const NavigationAction&) = default;
+NavigationAction& NavigationAction::operator=(NavigationAction&&) = default;
+
+NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, NavigationType type, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy, Event* event, const AtomicString& downloadAttribute)
+    : m_resourceRequest { resourceRequest }
+    , m_type { type }
+    , m_shouldOpenExternalURLsPolicy { shouldOpenExternalURLsPolicy }
+    , m_event { event }
+    , m_downloadAttribute { downloadAttribute }
+{
+}
+
 static NavigationType navigationType(FrameLoadType frameLoadType, bool isFormSubmission, bool haveEvent)
 {
     if (isFormSubmission)
@@ -48,68 +64,12 @@ static NavigationType navigationType(FrameLoadType frameLoadType, bool isFormSub
     return NavigationType::Other;
 }
 
-NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, NavigationType type, Event* event, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy, const AtomicString& downloadAttribute)
-    : m_resourceRequest(resourceRequest)
-    , m_type(type)
-    , m_event(event)
-    , m_userGestureToken(UserGestureIndicator::currentUserGesture())
-    , m_shouldOpenExternalURLsPolicy(shouldOpenExternalURLsPolicy)
-    , m_downloadAttribute(downloadAttribute)
-{
-}
-
-NavigationAction::NavigationAction()
-    : NavigationAction(ResourceRequest(), NavigationType::Other, nullptr, ShouldOpenExternalURLsPolicy::ShouldNotAllow, nullAtom)
-{
-}
-
-NavigationAction::NavigationAction(const ResourceRequest& resourceRequest)
-    : NavigationAction(resourceRequest, NavigationType::Other, nullptr, ShouldOpenExternalURLsPolicy::ShouldNotAllow, nullAtom)
-{
-}
-
-NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy)
-    : NavigationAction(resourceRequest, NavigationType::Other, nullptr, shouldOpenExternalURLsPolicy, nullAtom)
-{
-}
-
-NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, NavigationType type)
-    : NavigationAction(resourceRequest, type, nullptr, ShouldOpenExternalURLsPolicy::ShouldNotAllow, nullAtom)
-{
-}
-
-NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, NavigationType type, Event* event, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy)
-    : NavigationAction(resourceRequest, type, event, shouldOpenExternalURLsPolicy, nullAtom)
-{
-}
-
-NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, FrameLoadType frameLoadType, bool isFormSubmission)
-    : NavigationAction(resourceRequest, navigationType(frameLoadType, isFormSubmission, 0), nullptr, ShouldOpenExternalURLsPolicy::ShouldNotAllow, nullAtom)
-{
-}
-
-NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, NavigationType type, Event* event)
-    : NavigationAction(resourceRequest, type, event, ShouldOpenExternalURLsPolicy::ShouldNotAllow, nullAtom)
-{
-}
-
-NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, NavigationType type, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy)
-    : NavigationAction(resourceRequest, type, nullptr, shouldOpenExternalURLsPolicy, nullAtom)
-{
-}
-
-NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, FrameLoadType frameLoadType, bool isFormSubmission, Event* event)
-    : NavigationAction(resourceRequest, navigationType(frameLoadType, isFormSubmission, event), event, ShouldOpenExternalURLsPolicy::ShouldNotAllow, nullAtom)
-{
-}
-
-NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, FrameLoadType frameLoadType, bool isFormSubmission, Event* event, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy)
-    : NavigationAction(resourceRequest, navigationType(frameLoadType, isFormSubmission, event), event, shouldOpenExternalURLsPolicy, nullAtom)
-{
-}
-
 NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, FrameLoadType frameLoadType, bool isFormSubmission, Event* event, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy, const AtomicString& downloadAttribute)
-    : NavigationAction(resourceRequest, navigationType(frameLoadType, isFormSubmission, event), event, shouldOpenExternalURLsPolicy, downloadAttribute)
+    : m_resourceRequest { resourceRequest }
+    , m_type { navigationType(frameLoadType, isFormSubmission, !!event) }
+    , m_shouldOpenExternalURLsPolicy { shouldOpenExternalURLsPolicy }
+    , m_event { event }
+    , m_downloadAttribute { downloadAttribute }
 {
 }
 
index 3702dc5..0abb442 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2016 Apple Inc.  All rights reserved.
+ * Copyright (C) 2006-2017 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 
 #pragma once
 
-#include "Event.h"
 #include "FrameLoaderTypes.h"
-#include "URL.h"
 #include "ResourceRequest.h"
 #include "UserGestureIndicator.h"
 #include <wtf/Forward.h>
 
 namespace WebCore {
 
+class Event;
+
 class NavigationAction {
 public:
-    WEBCORE_EXPORT NavigationAction();
-    WEBCORE_EXPORT explicit NavigationAction(const ResourceRequest&);
-    WEBCORE_EXPORT NavigationAction(const ResourceRequest&, NavigationType);
-    WEBCORE_EXPORT NavigationAction(const ResourceRequest&, FrameLoadType, bool isFormSubmission);
-
-    NavigationAction(const ResourceRequest&, ShouldOpenExternalURLsPolicy);
-    NavigationAction(const ResourceRequest&, NavigationType, Event*);
-    NavigationAction(const ResourceRequest&, NavigationType, Event*, ShouldOpenExternalURLsPolicy);
-    NavigationAction(const ResourceRequest&, NavigationType, Event*, ShouldOpenExternalURLsPolicy, const AtomicString& downloadAttribute);
-    NavigationAction(const ResourceRequest&, NavigationType, ShouldOpenExternalURLsPolicy);
-    NavigationAction(const ResourceRequest&, FrameLoadType, bool isFormSubmission, Event*);
-    NavigationAction(const ResourceRequest&, FrameLoadType, bool isFormSubmission, Event*, ShouldOpenExternalURLsPolicy);
-    NavigationAction(const ResourceRequest&, FrameLoadType, bool isFormSubmission, Event*, ShouldOpenExternalURLsPolicy, const AtomicString& downloadAttribute);
+    WEBCORE_EXPORT explicit NavigationAction(const ResourceRequest& = { }, NavigationType = NavigationType::Other, ShouldOpenExternalURLsPolicy = ShouldOpenExternalURLsPolicy::ShouldNotAllow, Event* = nullptr, const AtomicString& downloadAttribute = nullAtom);
+    NavigationAction(const ResourceRequest&, FrameLoadType, bool isFormSubmission, Event* = nullptr, ShouldOpenExternalURLsPolicy = ShouldOpenExternalURLsPolicy::ShouldNotAllow, const AtomicString& downloadAttribute = nullAtom);
+
+    WEBCORE_EXPORT ~NavigationAction();
+
+    WEBCORE_EXPORT NavigationAction(const NavigationAction&);
+    NavigationAction(NavigationAction&&);
+
+    NavigationAction& operator=(const NavigationAction&);
+    NavigationAction& operator=(NavigationAction&&);
 
     NavigationAction copyWithShouldOpenExternalURLsPolicy(ShouldOpenExternalURLsPolicy) const;
 
@@ -72,10 +69,10 @@ public:
 
 private:
     ResourceRequest m_resourceRequest;
-    NavigationType m_type { NavigationType::Other };
+    NavigationType m_type;
+    ShouldOpenExternalURLsPolicy m_shouldOpenExternalURLsPolicy;
     RefPtr<Event> m_event;
-    RefPtr<UserGestureToken> m_userGestureToken;
-    ShouldOpenExternalURLsPolicy m_shouldOpenExternalURLsPolicy { ShouldOpenExternalURLsPolicy::ShouldNotAllow };
+    RefPtr<UserGestureToken> m_userGestureToken { UserGestureIndicator::currentUserGesture() };
     AtomicString m_downloadAttribute;
 };
 
index 4eae7b6..54af1f2 100644 (file)
@@ -35,6 +35,7 @@
 #include "ContentSecurityPolicy.h"
 #include "DOMWindow.h"
 #include "DocumentLoader.h"
+#include "Event.h"
 #include "EventNames.h"
 #include "FormState.h"
 #include "Frame.h"
index c35b0ef..4fa56e8 100644 (file)
@@ -37,6 +37,7 @@
 
 #include "Document.h"
 #include "DocumentLoader.h"
+#include "Event.h"
 #include "EventNames.h"
 #include "Frame.h"
 #include "PerformanceEntry.h"
index bf358dc..c01b38b 100644 (file)
@@ -34,6 +34,7 @@
 #include "CapturingInputCursor.h"
 #include "DOMWindow.h"
 #include "DocumentLoader.h"
+#include "Event.h"
 #include "Frame.h"
 #include "FrameTree.h"
 #include "InspectorInstrumentation.h"