OOPIFs: Fixing submitting forms targeting a remote frame.

third_party/WebKit/Source/core/html/HTMLFormElement.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 18 matching lines @@
 #include "bindings/core/v8/ScriptEventListener.h"
 #include "core/HTMLNames.h"
 #include "core/dom/Attribute.h"
 #include "core/dom/Document.h"
 #include "core/dom/ElementTraversal.h"
 #include "core/dom/NodeListsNodeData.h"
 #include "core/events/Event.h"
 #include "core/events/ScopedEventQueue.h"
 #include "core/frame/LocalDOMWindow.h"
 #include "core/frame/LocalFrame.h"
+#include "core/frame/RemoteFrame.h"
 #include "core/frame/UseCounter.h"
 #include "core/frame/csp/ContentSecurityPolicy.h"
 #include "core/html/HTMLCollection.h"
 #include "core/html/HTMLDialogElement.h"
 #include "core/html/HTMLFormControlsCollection.h"
 #include "core/html/HTMLImageElement.h"
 #include "core/html/HTMLInputElement.h"
 #include "core/html/HTMLObjectElement.h"
 #include "core/html/RadioNodeList.h"
 #include "core/html/forms/FormController.h"
 #include "core/inspector/ConsoleMessage.h"
+#include "core/loader/FormSubmission.h"
 #include "core/loader/FrameLoader.h"
 #include "core/loader/FrameLoaderClient.h"
 #include "core/loader/MixedContentChecker.h"
 #include "core/loader/NavigationScheduler.h"
 #include "platform/UserGestureIndicator.h"
 #include "public/platform/WebInsecureRequestPolicy.h"
 #include "wtf/text/AtomicString.h"
 #include <limits>
 
 namespace blink {
@@ skipping 353 matching lines @@
     } else {
         submission->clearTarget();
     }
     if (!targetFrame->host())
         return;
 
     UseCounter::count(document(), UseCounter::FormsSubmitted);
     if (MixedContentChecker::isMixedFormAction(document().frame(), submission->action()))
         UseCounter::count(document().frame(), UseCounter::MixedContentFormsSubmitted);
 
-    // FIXME: Plumb form submission for remote frames.
-    if (targetFrame->isLocalFrame())
+    if (targetFrame->isLocalFrame()) {
         toLocalFrame(targetFrame)->navigationScheduler().scheduleFormSubmission(&document(), submission);
+    } else {
+        FrameLoadRequest frameLoadRequest = submission->createFrameLoadRequest(&document());
+        toRemoteFrame(targetFrame)->navigate(frameLoadRequest);

[dcheng, 2016/06/12 05:50:29]

Crazy idea: is it possible to just call Frame::navigate() for both paths here?
We can just always do this in a timer so we still get the scheduled effect.

[Łukasz Anforowicz, 2016/06/13 17:13:42]

Hmmm.  This does sound like something we should at least seriously attempt.  At
the same time I'd prefer to keep the current CL as-is - the alternative approach
has higher risk of changing behavior for local frames (not really touched in the
current CL) and so containing the changes in a separate CL seems desirable (to
enable evaluating the impact in separation + to enable reverting in separation).
 I also have hidden motivation for keeping the current CL as-is: 1) this will
help me rebase and land the OpenURL CLs and 2) I want to minimize the risk the
POST work won't land before my vacation starts next week... :-/

I see the following aspects of behavior we would need to ensure we are
preserving (if replacing navigationScheduler for the local case with an ad-hoc
timer + virtual Frame::navigate call):

- NavigationScheduler::mustReplaceCurrentItem(LocalFrame* targetFrame) is
currently private and therefore only callable via the current path.  We would
need to figure out how to duplicate the behavior outside of the current code
path without duplicating the code.  OTOH, I do note that calculation of
|replaceCurrentItem| for remote frames seems sketchy in the current CL (well,
there is no calculation - we depend on replaceCurrentItem being set by default
to false by the constructors of FrameLoadRequest).

- It is not immediately clear how to preserve maybeLogScheduledNavigationClobber
outside of navigationScheduler path.

- I don't quite understand the significance of instantiating
OwnPtr<UserGestureIndicator> gestureIndicator = createUserGestureIndicator(); in
ScheduledFormSubmission::fire (as opposed to just calling static methods of
UserGestureIndicator, without constructing an instance).

Additionally - we probably should audit other scheduled navigations in
core/loader/NavigationScheduler.cpp and see if they should also be potentially
replaced with a timer-based call to Frame::navigate (i.e. if these navigations
can target a remote frame).

FWIW, I've added a TODO here...

+    }
 }
 
 void HTMLFormElement::reset()
 {
     LocalFrame* frame = document().frame();
     if (m_isInResetFunction || !frame)
         return;
 
     m_isInResetFunction = true;
 
@@ skipping 340 matching lines @@
 {
     for (const auto& control : associatedElements()) {
         if (!control->isFormControlElement())
             continue;
         if (toHTMLFormControlElement(control)->canBeSuccessfulSubmitButton())
             toHTMLFormControlElement(control)->pseudoStateChanged(CSSSelector::PseudoDefault);
     }
 }
 
 } // namespace blink