Bug 91707

Created attachment 153176 [details] Patch

Dropping the r?, as this can't land as-is. I still need to add the files to the xcode project, and I'm having a devil of a time doing so. Still, if you wouldn't mind having a look, Adam, I'd appreciate it. This is more or less the same patch I've been running by you on GitHub for a while now, rebased on top of trunk.

(In reply to comment #2) > Dropping the r?, as this can't land as-is. I still need to add the files to the xcode project, and I'm having a devil of a time doing so. You basically need a Mac for that. > Still, if you wouldn't mind having a look, Adam, I'd appreciate it. This is more or less the same patch I've been running by you on GitHub for a while now, rebased on top of trunk. Sure.

Comment on attachment 153176 [details] Patch Attachment 153176 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13279798

Comment on attachment 153176 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153176&action=review Looks pretty good. Weaving the sendReport everywhere is kind of ugly. Maybe using an enum would have better readability at the call sites? > Source/WebCore/page/DOMSecurityPolicy.idl:30 > + InterfaceName=securityPolicy securityPolicy -> SecurityPolicy

Thanks! If/when I can get WebKit checked out on my laptop, I'll keep fiddling with the xcode file. (In reply to comment #5) > (From update of attachment 153176 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=153176&action=review > > Looks pretty good. Weaving the sendReport everywhere is kind of ugly. Maybe using an enum would have better readability at the call sites? It is ugly. While rebasing I was wondering again whether it made sense. I think it does, but it's ugly. I'm not sure an enum makes it less ugly, but it probably would make it more readable. > > > Source/WebCore/page/DOMSecurityPolicy.idl:30 > > + InterfaceName=securityPolicy > > securityPolicy -> SecurityPolicy Will do.

In my humble opinion, it looks you missed to touch below file for GTK port. http://trac.webkit.org/browser/trunk/Source/WebCore/bindings/gobject/GNUmakefile.am For example, as below, + DerivedSources/webkit/WebKitDOMDOMSecurityPolicy.h \ + DerivedSources/webkit/WebKitDOMDOMSecurityPolicyPrivate.h \ @@ -277,6 +279,7 @@ webkitgtk_built_h_api += \ DerivedSources/webkit/WebKitDOMDocumentFragment.h \ DerivedSources/webkit/WebKitDOMDocumentType.h \ DerivedSources/webkit/WebKitDOMDOMImplementation.h \ + DerivedSources/webkit/WebKitDOMDOMSecurityPolicy.h \

Created attachment 153285 [details] Adam's feedback + GTK, still no XCode.

(In reply to comment #7) > In my humble opinion, it looks you missed to touch below file for GTK port. > > http://trac.webkit.org/browser/trunk/Source/WebCore/bindings/gobject/GNUmakefile.am > > For example, as below, > > + DerivedSources/webkit/WebKitDOMDOMSecurityPolicy.h \ > + DerivedSources/webkit/WebKitDOMDOMSecurityPolicyPrivate.h \ > > @@ -277,6 +279,7 @@ webkitgtk_built_h_api += \ > DerivedSources/webkit/WebKitDOMDocumentFragment.h \ > DerivedSources/webkit/WebKitDOMDocumentType.h \ > DerivedSources/webkit/WebKitDOMDOMImplementation.h \ > + DerivedSources/webkit/WebKitDOMDOMSecurityPolicy.h \ Thank you! I've added these to the file (though it looks like I need to add `DerivedSources/webkit/WebKitDOMDOMSecurityPolicy.cpp`, based on what others have done). Can you take a look at the current patch and verify that I've done it correctly?

Comment on attachment 153285 [details] Adam's feedback + GTK, still no XCode. Attachment 153285 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13272889

Created attachment 153517 [details] AdGTK again. I hate XCode.

(In reply to comment #11) > Created an attachment (id=153517) [details] > AdGTK again. I hate XCode. I've been kicking around with XCode for a few hours, and all I have to show for it are an ever-widening variety of error messages. It's driving me nuts. Can you give me some pointers as to exactly where I need to add these files? Or should I hop over to webkit-dev for help?

> I've been kicking around with XCode for a few hours, and all I have to show for it are an ever-widening variety of error messages. It's driving me nuts. What do the error messages look like?

(In reply to comment #13) > > I've been kicking around with XCode for a few hours, and all I have to show for it are an ever-widening variety of error messages. It's driving me nuts. > > What do the error messages look like? I started with: In file included from /Users/mkwst/Repositories/webkit/WebKitBuild/Debug/DerivedSources/WebCore/DOMDocument.mm:48: /Users/mkwst/Repositories/webkit/WebKitBuild/Debug/DerivedSources/WebCore/DOMDOMSecurityPolicyInternal.h:27:9: fatal error: 'WebCore/DOMDOMSecurityPolicy.h' file not found [2] #import <WebCore/DOMDOMSecurityPolicy.h> And the most recent failure at resolving that was: PBXCp /Users/mkwst/Repositories/webkit/WebKitBuild/Debug/DerivedSources/WebCore/../../../../../../WebKitBuild/Debug/DerivedSources/WebCore/DOMDOMSecurityPolicy.h /Users/mkwst/Repositories/webkit/WebKitBuild/Debug/WebCore.framework/Versions/A/PrivateHeaders/DOMDOMSecurityPolicy.h cd /Users/mkwst/Repositories/webkit/Source/WebCore builtin-copy -exclude .DS_Store -exclude CVS -exclude .svn -exclude .git -exclude .hg -strip-debug-symbols -resolve-src-symlinks /Users/mkwst/Repositories/webkit/WebKitBuild/Debug/DerivedSources/WebCore/../../../../../../WebKitBuild/Debug/DerivedSources/WebCore/DOMDOMSecurityPolicy.h /Users/mkwst/Repositories/webkit/WebKitBuild/Debug/WebCore.framework/Versions/A/PrivateHeaders error: /Users/mkwst/Repositories/webkit/WebKitBuild/Debug/DerivedSources/WebCore/../../../../../../WebKitBuild/Debug/DerivedSources/WebCore/DOMDOMSecurityPolicy.h: No such file or directory Basically, adding the three files I actually wrote seems to be no problem. I'm failing to add the generated files.

DOMDOMSecurityPolicy.h is related to the Objective-C bindings. You might need to add the header to bindings/objc/DerivedSources in the Xcode project file.

(In reply to comment #15) > DOMDOMSecurityPolicy.h is related to the Objective-C bindings. You might need to add the header to bindings/objc/DerivedSources in the Xcode project file. I love you. :) It looks like it's compiling. If it continues to look like it's compiling, I'll upload a patch for review. Thanks!

Created attachment 153592 [details] XCode is happy. I think.

(In reply to comment #17) > Created an attachment (id=153592) [details] > XCode is happy. I think. Friendly ping. :) This patch now compiles cleanly under XCode, and I've swapped out the boolean flag for an enum. Would you mind taking another look, Adam? Thanks!

Comment on attachment 153592 [details] XCode is happy. I think. View in context: https://bugs.webkit.org/attachment.cgi?id=153592&action=review > Source/WebCore/page/DOMSecurityPolicy.cpp:37 > + extra blank line here. > Source/WebCore/page/DOMSecurityPolicy.h:63 > + bool allowsConnectionTo(String url) const; > + bool allowsFontFrom(String url) const; > + bool allowsFrameFrom(String url) const; > + bool allowsImageFrom(String url) const; > + bool allowsMediaFrom(String url) const; > + bool allowsObjectFrom(String url) const; > + bool allowsScriptFrom(String url) const; > + bool allowsStyleFrom(String url) const; These should all be const String& > Source/WebCore/page/DOMSecurityPolicy.idl:30 > + InterfaceName=securityPolicy securityPolicy -> SecurityPolicy

Created attachment 154574 [details] Patch for landing. Thanks, Adam.

Comment on attachment 154574 [details] Patch for landing. Aren't you a committer?

You can set the commit-queue flag yourself. :)

Maybe your paperwork hasn't gone through yet?

Comment on attachment 154574 [details] Patch for landing. Clearing flags on attachment: 154574 Committed r123722: <http://trac.webkit.org/changeset/123722>

All reviewed patches have been landed. Closing bug.