Bug 45638 – Move mixed content logic out of FrameLoader

• Component: New Bugs
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

Move mixed content logic out of FrameLoader

[Eric Seidel (no email)]

Created attachment 67364 [details]
Patch

[Adam Barth]

Comment on attachment 67364 [details]
Patch

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

You forgot to add the new files.

[Early Warning System Bot]

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

[Eric Seidel (no email)]

Created attachment 67366 [details]
Patch

[Adam Barth]

Comment on attachment 67366 [details]
Patch

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

> WebCore/loader/MixedContentChecker.cpp:63
> +inline CString asUTF8(const KURL& url)
> +{
> +    return url.string().utf8();
> +}
static?

> WebCore/loader/MixedContentChecker.cpp:65
> +void MixedContentChecker::checkIfDisplayInsecureContent(SecurityOrigin* context, const KURL& url) const
We usually call SecurityOrigin objects securityOrigin.  Usually "context" refers to a ScriptExecutionContext.

> WebCore/loader/MixedContentChecker.h:49
> +    void checkIfDisplayInsecureContent(SecurityOrigin* context, const KURL&) const;
> +    void checkIfRunInsecureContent(SecurityOrigin* context, const KURL&) const;
The parameter name for SecurityOrigin isn't needed here.

[Eric Seidel (no email)]

Attachment 67366 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3958419

[Eric Seidel (no email)]

Created attachment 67377 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 67377 [details]
Patch for landing

Rejecting patch 67377 from commit-queue.

Failed to run "['WebKitTools/Scripts/build-webkit', '--debug']" exit_code: 1
Last 500 characters of output:
uild/WebKit.build/Debug/WebKit.build/Objects-normal/i386/WebTextCompletionController.o /Users/eseidel/Projects/CommitQueue/WebKit/mac/WebView/WebTextCompletionController.mm normal i386 objective-c++ com.apple.compilers.gcc.4_2
	Distributed-CompileC /Users/eseidel/Projects/CommitQueue/WebKitBuild/WebKit.build/Debug/WebKit.build/Objects-normal/i386/WebPDFView.o /Users/eseidel/Projects/CommitQueue/WebKit/mac/WebView/WebPDFView.mm normal i386 objective-c++ com.apple.compilers.gcc.4_2
(31 failures)


Full output: http://queues.webkit.org/results/3909444

[Eric Seidel (no email)]

Created attachment 67424 [details]
Patch for landing

[Eric Seidel (no email)]

Created attachment 67432 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 67432 [details]
Patch for landing

Rejecting patch 67432 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--force']" exit_code: 1
Last 500 characters of output:
lines).
Hunk #8 FAILED at 20569.
Hunk #9 FAILED at 23044.
2 out of 9 hunks FAILED -- saving rejects to file WebCore/WebCore.xcodeproj/project.pbxproj.rej
patching file WebCore/loader/CachedResourceLoader.cpp
patching file WebCore/loader/FrameLoader.cpp
patching file WebCore/loader/FrameLoader.h
patching file WebCore/loader/MainResourceLoader.cpp
patching file WebCore/loader/MixedContentChecker.cpp
patching file WebCore/loader/MixedContentChecker.h
patching file WebCore/loader/SubframeLoader.cpp

Full output: http://queues.webkit.org/results/4034002

[Eric Seidel (no email)]

I'm not actively working on this.

[Eric Seidel (no email)]

CCing nate in case the spirit moves him.

[Mike West]

I filed https://bugs.webkit.org/show_bug.cgi?id=97979 to fix the FIXME you noted in the last patch on this bug regarding the lack of line number and URL in the console message for mixed content. I'll take this as a prereq for that, if you don't mind; it should be straightforward to get running again on ToT.

[Eric Seidel (no email)]

Please!  Thank you!

[Mike West]

Created attachment 168596 [details]
Patch

[Mike West]

Let's see if I'm capable of correctly adding files with XCode...

Beyond the new call sites, the only substantive change I made to the patch was to rename the methods from "checkIf*InsecureContent" to "can*InsecureContent". That seems to better represent what the boolean return value actually means. Everything else is mostly as you left it, Eric.

[Build Bot]

Comment on attachment 168596 [details]
Patch

Attachment 168596 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14292581

[Eric Seidel (no email)]

Comment on attachment 168596 [details]
Patch

LGTM.  I think Adam had also r+'d my original patch.  You'll need to re-upload and get the mac-ews building before landing.

[Mike West]

Created attachment 169142 [details]
XCode.

[Mike West]

Created attachment 169179 [details]
EWS is ignoring the previous patch. :/

[Mike West]

Created attachment 169184 [details]
XCode.

[Mike West]

Created attachment 169225 [details]
It compiles on OSX locally. Honest.

[Mike West]

Fingers crossed, but it looks like the magical incantations to get things working on OSX locally include adding " settings = {ATTRIBUTES = (Private, ); };" to one of the instances of the header file in the project file. I came across it by pure copy/paste cargo culting... I have no idea what it does, nor do I have any idea how I could have added it via the XCode interface. Can one of you explain it to me? :)

Regardless, let's hope it works on the bot.

[Adam Barth]

> Can one of you explain it to me? :)

In the apple-mac build, each of the major components (WTF, JavaScriptCore, WebCore, WebKit, WebKit2) are built separately as frameworks.  When, for example, WebKit2 includes a header from WebCore, it needs to be able to find that header in WebCore.framework.  By default, headers are not copied into the framework.  Marking a header as Private means it is copied in the PrivateHeaders directory in the framework, which means it is available at build-time but is not shipped in productions builds of Mac OS X.

There is a checkbox buried in the Xcode UI for classifying headers as project, private, or public.  The default is project, which means it is only visible within the project (i.e., not copied into the framework).  Private means it goes in the PrivateHeaders directory (see above).  Public means it goes in the Headers directory and is available for third-party developers to use as part of the production Mac OS X build.

[Mike West]

Comment on attachment 169225 [details]
It compiles on OSX locally. Honest.

Thanks Adam, that makes sense. :)

[WebKit Review Bot]

Comment on attachment 169225 [details]
It compiles on OSX locally. Honest.

Clearing flags on attachment: 169225

Committed r131704: <http://trac.webkit.org/changeset/131704>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.