Bug 42293 – [Qt] Remove FrameLoaderClientQt::webFrame() to use NetworkingContext to get the WebFrame to avoid layering violations

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Jesus Sanchez-Palencia]

After we have something like a PlatformWebFrame and a FrameLoaderClient::platformWebFrame we should remove all layering violations related to this.

[Jesus Sanchez-Palencia]

Created attachment 61574 [details]
patch

[Antonio Gomes]

Good you are looking into it!

An yet-not-perfect-alternate-approach would be replacing the use of FrameLoaderClientQt in WebCore by ...

QWebFrame* QWebFramePrivate::core(Frame*)

... method. Given a Frame, it returns the associated QWebFrame.

Of course, you'd be eliminating all direct accesses to FrameLoaderClientQt from WebCore *but* including qwebframe_p.h.

Again, it is not perfect, but works better anyways to me.

[Antonio Gomes]

(In reply to comment #2)
> QWebFrame* QWebFramePrivate::core(Frame*)

s/core/kit

[Jesus Sanchez-Palencia]

After NetworkingContext (bug 42292) is reviewed and landed, we should probably review the need of having FrameLoaderClientQt::webFrame().

[Diego Gonzalez]

Created attachment 67606 [details]
Patch

[Early Warning System Bot]

Attachment 67606 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4019008

[Kenneth Rohde Christiansen]

Comment on attachment 67606 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=67606&action=prettypatch

> WebKit/qt/Api/qwebframe.cpp:898
> +            QWebFrame* webFrame = static_cast<QWebFrame*>(loader->networkingContext()->originatingObject());
qobject_cast?

> WebKit/qt/Api/qwebframe.cpp:1457
> +    return static_cast<QWebFrame*>(coreFrame->loader()->networkingContext()->originatingObject());
qobject_cast?

> WebKit/qt/Api/qwebpage.cpp:1921
> +    return static_cast<QWebFrame*>(frame->loader()->networkingContext()->originatingObject());
qobject_cast?

> WebKit/qt/WebCoreSupport/ChromeClientQt.cpp:298
> +    QWebFrame* webFrame = static_cast<QWebFrame*>(f->loader()->networkingContext()->originatingObject());
here

> WebKit/qt/WebCoreSupport/ChromeClientQt.cpp:305
> +    QWebFrame* webFrame = static_cast<QWebFrame*>(f->loader()->networkingContext()->originatingObject());
here as well

> WebKit/qt/WebCoreSupport/ChromeClientQt.cpp:312
> +    QWebFrame* webFrame = static_cast<QWebFrame*>(f->loader()->networkingContext()->originatingObject());
and here :-)

[Diego Gonzalez]

Created attachment 67670 [details]
Patch v2

[Early Warning System Bot]

Attachment 67670 [details] did not build on qt:
Build output: http://queues.webkit.org/results/3997020

[Diego Gonzalez]

Comment on attachment 67670 [details]
Patch v2

Wrong patch. :(

[Antonio Gomes]

since static_cast is faster , in this case it wont't fail, and he is not null-checking anyways, I thought it was ok as is.

no strong preference.

(In reply to comment #7)
> (From update of attachment 67606 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=67606&action=prettypatch
> 
> > WebKit/qt/Api/qwebframe.cpp:898
> > +            QWebFrame* webFrame = static_cast<QWebFrame*>(loader->networkingContext()->originatingObject());
> qobject_cast?
> 
> > WebKit/qt/Api/qwebframe.cpp:1457
> > +    return static_cast<QWebFrame*>(coreFrame->loader()->networkingContext()->originatingObject());
> qobject_cast?
> 
> > WebKit/qt/Api/qwebpage.cpp:1921
> > +    return static_cast<QWebFrame*>(frame->loader()->networkingContext()->originatingObject());
> qobject_cast?
> 
> > WebKit/qt/WebCoreSupport/ChromeClientQt.cpp:298
> > +    QWebFrame* webFrame = static_cast<QWebFrame*>(f->loader()->networkingContext()->originatingObject());
> here
> 
> > WebKit/qt/WebCoreSupport/ChromeClientQt.cpp:305
> > +    QWebFrame* webFrame = static_cast<QWebFrame*>(f->loader()->networkingContext()->originatingObject());
> here as well
> 
> > WebKit/qt/WebCoreSupport/ChromeClientQt.cpp:312
> > +    QWebFrame* webFrame = static_cast<QWebFrame*>(f->loader()->networkingContext()->originatingObject());
> and here :-)

[Diego Gonzalez]

Created attachment 67672 [details]
Patch v3

[WebKit Commit Bot]

Comment on attachment 67672 [details]
Patch v3

Clearing flags on attachment: 67672

Committed r67608: <http://trac.webkit.org/changeset/67608>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.