Bug 12311 – Implement frame loading for Qt. Clean up FrameLoader a bit

• Component: Page Loading
• Status: RESOLVED FIXED

[Lars Knoll]

The patch implements support for creation and loading of subframes in the Qt build. At the same time it cleans up some of the remaining platform dependent code in FrameLoader. It moves some code that is platform independent into loadSubFrame and adds stubs for most of the platform dependent FrameLoader methods to the FrameLoaderClient (so that FrameLoader can become 100% platform independent one day). On the Mac side it only cleans up a little, but makes it possible to move quite a bit of code from FrameMac and the WebFrameBridge to WebFrameLoaderClient.

The code is untested on th Mac, so someone needs to give it a try to see if it compiles and works.

[Lars Knoll]

Created attachment 12533 [details]
see bug description

[Adam Roben (:aroben)]

Comment on attachment 12533 [details]
see bug description

 #include "HTMLFormElement.h"
 #include "HTMLNames.h"
 #include "HTMLObjectElement.h"
+#include "HTMLFrameElement.h"

   #includes should be alphabetical.

+    if (ownerElement->hasTagName(frameTag) || ownerElement->hasTagName(iframeTag)) {
+        HTMLFrameElement* o = static_cast<HTMLFrameElement*>(ownerElement);

   I know this isn't new code, but it seems like this cast should really be to HTMLFrameElementBase, since that's what both HTMLFrameElement and HTMLIFrameElement derive from. Also, HTMLFrameElement and HTMLIFrameElement are the only things that can be HTMLFrameOwnerElements, so I'm not sure what value the if has.

     class ResourceRequest;
     class ResourceResponse;
     class String;
+    class HTMLFrameOwnerElement;
+    class Widget;
+    class IntSize;

   Please alphabetize these forward declarations.

+        virtual Frame *createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
+                                   const String& referrer, bool allowsScrolling, int marginWidth, int marginHeight) = 0;

   The * should be next to Frame, and the 'ownerElement' parameter name is unnecessary. (And ditto in WebFrameLoaderClient.h)

+    // stubbed out until the code is moved here from the FrameBridge, and FrameLoader looses the createFrame() method

    Typo: 'looses' => 'loses'

   I think it would be good for Darin to have a look at this before checking in.

[Adam Roben (:aroben)]

Comment on attachment 12533 [details]
see bug description

This patch also needs to update SVGEmptyFrameLoaderClient in SVGImageEmptyClients.h

[Lars Knoll]

Created attachment 12551 [details]
new version

[Adam Roben (:aroben)]

Created attachment 12552 [details]
Fixes Lars' patch to compile on Mac

This patch fixes Lars' latest patch to compile on Mac (changes to WebFrameLoaderClient.(h|mm) were made).

[Adam Roben (:aroben)]

Comment on attachment 12552 [details]
Fixes Lars' patch to compile on Mac

I've confirmed that this patch causes no layout test regressions on Mac.

[Darin Adler]

Comment on attachment 12552 [details]
Fixes Lars' patch to compile on Mac

I don't think this is a good way to handle the Mac side. If the functions still exist on the bridge that's fine. But they should be called through the frame loader client. The client can call to the bridge.

[Lars Knoll]

I agree, and that was the basic idea behind the change. But it was hard to change this without having a Mac at hand. Fortunately I got an iMac at work on Friday. 

So I can now move the Mac side over to the client as well. I'll prepare a new patch on Monday.

Lars

[Lars Knoll]

Created attachment 12610 [details]
new patch refactoring also the Mac code

[Lars Knoll]

My last patch compiles on the Mac and passes the regression tests. It makes Frames work for the Qt build, although it still has some smaller issues there (failing some frame related dom tests).

[Maciej Stachowiak]

Comment on attachment 12610 [details]
new patch refactoring also the Mac code

r=me but please fix the following:

1) "pieced" in the ChangeLog should be "pieces"
2) The below should use HTMLFrameElementBase.

+        HTMLFrameElement* o = static_cast<HTMLFrameElement*>(ownerElement);

Also please test Java since the LayoutTests don't cover it.

[Maciej Stachowiak]

Comment on attachment 12552 [details]
Fixes Lars' patch to compile on Mac

removing review flag and obsoleting

[David Kilzer (:ddkilzer)]

Please include Bugzilla bug numbers in ChangeLog entries in the future.  Thanks!

Fixed in r19042.