Bug 45638 - Move mixed content logic out of FrameLoader
Summary: Move mixed content logic out of FrameLoader
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Mike West
URL:
Keywords:
Depends on:
Blocks: 97979
  Show dependency treegraph
 
Reported: 2010-09-12 23:54 PDT by Eric Seidel (no email)
Modified: 2012-10-17 22:09 PDT (History)
10 users (show)

See Also:


Attachments
Patch (18.23 KB, patch)
2010-09-12 23:59 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch (24.58 KB, patch)
2010-09-13 00:10 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (24.61 KB, patch)
2010-09-13 01:37 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (24.65 KB, patch)
2010-09-13 10:17 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (24.64 KB, patch)
2010-09-13 10:59 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch (27.32 KB, patch)
2012-10-14 15:46 PDT, Mike West
no flags Details | Formatted Diff | Diff
XCode. (31.11 KB, patch)
2012-10-17 02:47 PDT, Mike West
no flags Details | Formatted Diff | Diff
EWS is ignoring the previous patch. :/ (31.11 KB, patch)
2012-10-17 07:09 PDT, Mike West
no flags Details | Formatted Diff | Diff
XCode. (29.47 KB, patch)
2012-10-17 07:54 PDT, Mike West
no flags Details | Formatted Diff | Diff
It compiles on OSX locally. Honest. (29.42 KB, patch)
2012-10-17 11:38 PDT, Mike West
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-09-12 23:54:47 PDT
Move mixed content logic out of FrameLoader
Comment 1 Eric Seidel (no email) 2010-09-12 23:59:01 PDT
Created attachment 67364 [details]
Patch
Comment 2 Adam Barth 2010-09-13 00:00:32 PDT
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.
Comment 3 Early Warning System Bot 2010-09-13 00:08:19 PDT
Attachment 67364 [details] did not build on qt:
Build output: http://queues.webkit.org/results/3960422
Comment 4 Eric Seidel (no email) 2010-09-13 00:10:20 PDT
Created attachment 67366 [details]
Patch
Comment 5 Adam Barth 2010-09-13 00:14:11 PDT
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.
Comment 6 Eric Seidel (no email) 2010-09-13 00:32:43 PDT
Attachment 67366 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3958419
Comment 7 Eric Seidel (no email) 2010-09-13 01:37:56 PDT
Created attachment 67377 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2010-09-13 03:09:14 PDT
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
Comment 9 Eric Seidel (no email) 2010-09-13 10:17:04 PDT
Created attachment 67424 [details]
Patch for landing
Comment 10 Eric Seidel (no email) 2010-09-13 10:59:36 PDT
Created attachment 67432 [details]
Patch for landing
Comment 11 WebKit Commit Bot 2010-09-13 19:05:18 PDT
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
Comment 12 Eric Seidel (no email) 2011-06-03 07:37:08 PDT
I'm not actively working on this.
Comment 13 Eric Seidel (no email) 2012-10-09 13:30:55 PDT
CCing nate in case the spirit moves him.
Comment 14 Mike West 2012-10-14 13:14:31 PDT
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.
Comment 15 Eric Seidel (no email) 2012-10-14 14:33:13 PDT
Please!  Thank you!
Comment 16 Mike West 2012-10-14 15:46:55 PDT
Created attachment 168596 [details]
Patch
Comment 17 Mike West 2012-10-14 15:49:28 PDT
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.
Comment 18 Build Bot 2012-10-14 16:20:35 PDT
Comment on attachment 168596 [details]
Patch

Attachment 168596 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14292581
Comment 19 Eric Seidel (no email) 2012-10-14 16:24:35 PDT
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.
Comment 20 Mike West 2012-10-17 02:47:38 PDT
Created attachment 169142 [details]
XCode.
Comment 21 Mike West 2012-10-17 07:09:48 PDT
Created attachment 169179 [details]
EWS is ignoring the previous patch. :/
Comment 22 Mike West 2012-10-17 07:54:08 PDT
Created attachment 169184 [details]
XCode.
Comment 23 Mike West 2012-10-17 11:38:25 PDT
Created attachment 169225 [details]
It compiles on OSX locally. Honest.
Comment 24 Mike West 2012-10-17 11:41:45 PDT
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.
Comment 25 Adam Barth 2012-10-17 12:05:06 PDT
> 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.
Comment 26 Mike West 2012-10-17 21:53:35 PDT
Comment on attachment 169225 [details]
It compiles on OSX locally. Honest.

Thanks Adam, that makes sense. :)
Comment 27 WebKit Review Bot 2012-10-17 22:09:15 PDT
Comment on attachment 169225 [details]
It compiles on OSX locally. Honest.

Clearing flags on attachment: 169225

Committed r131704: <http://trac.webkit.org/changeset/131704>
Comment 28 WebKit Review Bot 2012-10-17 22:09:21 PDT
All reviewed patches have been landed.  Closing bug.