Bug 75550 - Pass WebFrame instead of WebCore::Frame to the WebKit2 specific FrameNetworkingContext
Summary: Pass WebFrame instead of WebCore::Frame to the WebKit2 specific FrameNetworki...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexander Færøy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-04 08:24 PST by Alexander Færøy
Modified: 2012-01-05 09:02 PST (History)
5 users (show)

See Also:


Attachments
Patch (9.93 KB, patch)
2012-01-04 08:32 PST, Alexander Færøy
no flags Details | Formatted Diff | Diff
Patch (9.88 KB, patch)
2012-01-05 03:54 PST, Alexander Færøy
no flags Details | Formatted Diff | Diff
Patch (10.02 KB, patch)
2012-01-05 04:07 PST, Alexander Færøy
no flags Details | Formatted Diff | Diff
Patch (10.62 KB, patch)
2012-01-05 05:18 PST, Alexander Færøy
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Færøy 2012-01-04 08:24:49 PST
For the Qt port, we need access to the WebFrame::page()->pageID() to be able to implement HTTP authentication and SSL handling.
Comment 1 Alexander Færøy 2012-01-04 08:32:22 PST
Created attachment 121114 [details]
Patch

WIP patch. Uploaded to see what the EWS says :-)
Comment 2 Alexander Færøy 2012-01-04 12:02:00 PST
Adding Simon for review.

Patch worked for me locally on my Mac and it seems to pass EWS :-)
Comment 3 Kenneth Rohde Christiansen 2012-01-04 12:27:41 PST
Comment on attachment 121114 [details]
Patch

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

Please fix the changelog and bug title

> Source/WebKit2/ChangeLog:4
> +        [Qt] Pass WebKit::WebFrame instead of WebCore::Frame to WebFrameNetworkingContext
> +        https://bugs.webkit.org/show_bug.cgi?id=75550

This is not Qt only, so please remove the Qt part.

Pass WebFrame instead of WebCore::Frame to the WebKit2 specific FrameNetworkingContext

> Source/WebKit2/ChangeLog:9
> +        This patch changes the constructor of the various, port-specific,
> +        WebFrameNetworkingContext-classes to take a pointer to a
> +        WebKit::WebFrame instead of a WebCore::Frame. This is needed for the
> +        Qt port to implement HTTP authentication and SSL support.

You say it is port specific, so why do you need to change that for all ports... could you make this more clear?

Make the WebKit2 FrameNetworkingContext implementation store the WebFrame
instead of the WebCore::Frame as this is needed for the Qt port to implement a.o.
HTTP autentication and SSL Support.

As the WebFrameNetworkingContext is partly shared across all WebKit2 ports, all
port specific files have been modified.
Comment 4 Kenneth Rohde Christiansen 2012-01-04 12:28:15 PST
Comment on attachment 121114 [details]
Patch

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

> Source/WebKit2/WebProcess/WebCoreSupport/qt/WebFrameNetworkingContext.h:37
> +    ~WebFrameNetworkingContext();

Shouldn't this be virtual?
Comment 5 Alexander Færøy 2012-01-04 15:01:51 PST
Thanks for the review Kenneth.

I nailed down my related issue to the following bug in Qt: https://gist.github.com/1562660

Will talk with the Qt earth team tomorrow before proceeding with this.
Comment 6 Alexander Færøy 2012-01-05 03:54:41 PST
Created attachment 121254 [details]
Patch
Comment 7 Kenneth Rohde Christiansen 2012-01-05 04:04:11 PST
Comment on attachment 121254 [details]
Patch

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

> Source/WebKit2/WebProcess/WebCoreSupport/qt/WebFrameNetworkingContext.cpp:37
> +    // The page ID is needed later for HTTP authentication and SSL errors.
> +    m_originatingObject->setProperty("pageID", qulonglong(frame->page()->pageID()));

you do more in your patch than what you advertise
Comment 8 Alexander Færøy 2012-01-05 04:07:51 PST
Created attachment 121256 [details]
Patch
Comment 9 Simon Hausmann 2012-01-05 04:24:05 PST
Comment on attachment 121256 [details]
Patch

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

> Source/WebKit2/WebProcess/WebCoreSupport/qt/WebFrameNetworkingContext.cpp:42
> +    m_originatingObject->deleteLater();

Any particular reason for using deleteLater() here?

What about using an OwnPtr for m_originatingObject?
Comment 10 Alexander Færøy 2012-01-05 05:18:16 PST
Created attachment 121264 [details]
Patch
Comment 11 Alexander Færøy 2012-01-05 05:29:50 PST
Comment on attachment 121264 [details]
Patch

Removing CQ...
Comment 12 WebKit Review Bot 2012-01-05 09:02:38 PST
Comment on attachment 121264 [details]
Patch

Clearing flags on attachment: 121264

Committed r104168: <http://trac.webkit.org/changeset/104168>
Comment 13 WebKit Review Bot 2012-01-05 09:02:43 PST
All reviewed patches have been landed.  Closing bug.