- Revision
- 185033
- Author
- beid...@apple.com
- Date
- 2015-05-30 15:07:37 -0700 (Sat, 30 May 2015)
Log Message
Fix Windows tests broken by r185007.
https://bugs.webkit.org/show_bug.cgi?id=145472
Unreviewed. Discussed with Alexey Proskuryakov.
No new tests (Fixing the four broken ones should suffice).
The bug was that evaluating arguments in a method/constructor call has no guaranteed sequencing.
Clang seems to always do LTR, allowing the PassRefPtr to be evaluated as a bool in the call to
navigationType() before it is "consumed" by the PassRefPtr argument.
In Visual Studio the PassRefPtr consumption happened before the bool evaluation, meaning `false`
was always passed in to navigationType(), breaking the four tests.
The fix? Using PassRefPtr here was silly in the first place.
No ownership is being transferred.
Raw pointers it is.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadURL):
(WebCore::FrameLoader::loadPostRequest):
* loader/NavigationAction.cpp:
(WebCore::NavigationAction::NavigationAction):
* loader/NavigationAction.h: Nobody is actually transferring ownership of these Events.
Raw pointers will work just fine, thanks.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (185032 => 185033)
--- trunk/Source/WebCore/ChangeLog 2015-05-30 21:48:35 UTC (rev 185032)
+++ trunk/Source/WebCore/ChangeLog 2015-05-30 22:07:37 UTC (rev 185033)
@@ -1,3 +1,33 @@
+2015-05-30 Brady Eidson <beid...@apple.com>
+
+ Fix Windows tests broken by r185007.
+ https://bugs.webkit.org/show_bug.cgi?id=145472
+
+ Unreviewed. Discussed with Alexey Proskuryakov.
+
+ No new tests (Fixing the four broken ones should suffice).
+
+ The bug was that evaluating arguments in a method/constructor call has no guaranteed sequencing.
+
+ Clang seems to always do LTR, allowing the PassRefPtr to be evaluated as a bool in the call to
+ navigationType() before it is "consumed" by the PassRefPtr argument.
+
+ In Visual Studio the PassRefPtr consumption happened before the bool evaluation, meaning `false`
+ was always passed in to navigationType(), breaking the four tests.
+
+ The fix? Using PassRefPtr here was silly in the first place.
+ No ownership is being transferred.
+ Raw pointers it is.
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadURL):
+ (WebCore::FrameLoader::loadPostRequest):
+ * loader/NavigationAction.cpp:
+
+ (WebCore::NavigationAction::NavigationAction):
+ * loader/NavigationAction.h: Nobody is actually transferring ownership of these Events.
+ Raw pointers will work just fine, thanks.
+
2015-05-29 Joseph Pecoraro <pecor...@apple.com>
Unreviewed bindings test rebaseline after r185023.
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (185032 => 185033)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2015-05-30 21:48:35 UTC (rev 185032)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2015-05-30 22:07:37 UTC (rev 185033)
@@ -1230,7 +1230,7 @@
if (m_pageDismissalEventBeingDispatched != NoDismissal)
return;
- NavigationAction action(request, newLoadType, isFormSubmission, event);
+ NavigationAction action(request, newLoadType, isFormSubmission, event.get());
if (!targetFrame && !frameName.isEmpty()) {
policyChecker().checkNewWindowPolicy(action, request, formState.release(), frameName, [this, allowNavigationToInvalidURL, openerPolicy](const ResourceRequest& request, PassRefPtr<FormState> formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
@@ -2643,7 +2643,7 @@
workingResourceRequest.setHTTPContentType(contentType);
addExtraFieldsToRequest(workingResourceRequest, loadType, true);
- NavigationAction action(workingResourceRequest, loadType, true, event);
+ NavigationAction action(workingResourceRequest, loadType, true, event.get());
if (!frameName.isEmpty()) {
// The search for a target frame is done earlier in the case of form submission.
Modified: trunk/Source/WebCore/loader/NavigationAction.cpp (185032 => 185033)
--- trunk/Source/WebCore/loader/NavigationAction.cpp 2015-05-30 21:48:35 UTC (rev 185032)
+++ trunk/Source/WebCore/loader/NavigationAction.cpp 2015-05-30 22:07:37 UTC (rev 185033)
@@ -48,7 +48,7 @@
return NavigationType::Other;
}
-NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, NavigationType type, PassRefPtr<Event> event, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy)
+NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, NavigationType type, Event* event, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy)
: m_resourceRequest(resourceRequest)
, m_type(type)
, m_event(event)
@@ -77,7 +77,7 @@
{
}
-NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, NavigationType type, PassRefPtr<Event> event)
+NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, NavigationType type, Event* event)
: NavigationAction(resourceRequest, type, event, ShouldOpenExternalURLsPolicy::ShouldNotAllow)
{
}
@@ -87,7 +87,7 @@
{
}
-NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, FrameLoadType frameLoadType, bool isFormSubmission, PassRefPtr<Event> event)
+NavigationAction::NavigationAction(const ResourceRequest& resourceRequest, FrameLoadType frameLoadType, bool isFormSubmission, Event* event)
: NavigationAction(resourceRequest, navigationType(frameLoadType, isFormSubmission, event), event, ShouldOpenExternalURLsPolicy::ShouldNotAllow)
{
}
Modified: trunk/Source/WebCore/loader/NavigationAction.h (185032 => 185033)
--- trunk/Source/WebCore/loader/NavigationAction.h 2015-05-30 21:48:35 UTC (rev 185032)
+++ trunk/Source/WebCore/loader/NavigationAction.h 2015-05-30 22:07:37 UTC (rev 185033)
@@ -43,10 +43,10 @@
WEBCORE_EXPORT explicit NavigationAction(const ResourceRequest&);
WEBCORE_EXPORT NavigationAction(const ResourceRequest&, NavigationType);
WEBCORE_EXPORT NavigationAction(const ResourceRequest&, FrameLoadType, bool isFormSubmission);
- NavigationAction(const ResourceRequest&, NavigationType, PassRefPtr<Event>);
- NavigationAction(const ResourceRequest&, NavigationType, PassRefPtr<Event>, ShouldOpenExternalURLsPolicy);
+ NavigationAction(const ResourceRequest&, NavigationType, Event*);
+ NavigationAction(const ResourceRequest&, NavigationType, Event*, ShouldOpenExternalURLsPolicy);
NavigationAction(const ResourceRequest&, NavigationType, ShouldOpenExternalURLsPolicy);
- NavigationAction(const ResourceRequest&, FrameLoadType, bool isFormSubmission, PassRefPtr<Event>);
+ NavigationAction(const ResourceRequest&, FrameLoadType, bool isFormSubmission, Event*);
bool isEmpty() const { return m_resourceRequest.url().isEmpty(); }
