Bug 82826

Summary: Add security checks for iframe seamless
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: WebCore Misc.Assignee: Eric Seidel (no email) <eric>
Status: RESOLVED LATER    
Severity: Normal CC: abarth, ap, kkimdavenport, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 45950    
Attachments:
Description Flags
Patch
none
Patch for landing webkit.review.bot: commit-queue-

Description Eric Seidel (no email) 2012-03-31 01:19:37 PDT
Add security checks for iframe seamless
Comment 1 Eric Seidel (no email) 2012-03-31 01:20:20 PDT
Created attachment 134950 [details]
Patch
Comment 2 Eric Seidel (no email) 2012-03-31 01:28:05 PDT
We don't have to land this by itself, but I figure its easier to review in isolation.

Ideally i'd present this whole series as a git branch instead of patches. :)
Comment 3 Adam Barth 2012-03-31 01:32:44 PDT
Comment on attachment 134950 [details]
Patch

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

This is fine, but I'd prefer to land this together with tests.

> Source/WebCore/dom/Document.cpp:4755
> +static bool computeEligabilityForSeamless(Document* parent, Document* child)

I would have called this isEligabilityForSeamless

> Source/WebCore/dom/SecurityContext.h:70
> +    bool mayDisplaySeamlessWithParent() const { return m_mayDisplaySeamlessWithParent; }

Usually we use the word "can" rather than "may", although you're right that "may" is more grammatically correct.  IMHO, we should use "can" for consistency.

> Source/WebCore/dom/SecurityContext.h:85
> +    // Set in Document::initSecurityContext() at Document creation, per:

I'd remove this comment.
Comment 4 Alexey Proskuryakov 2012-04-01 00:25:22 PDT
> I would have called this isEligabilityForSeamless

isEligibleForSeamless?
Comment 5 Eric Seidel (no email) 2012-04-01 12:46:18 PDT
Created attachment 135011 [details]
Patch for landing
Comment 6 WebKit Review Bot 2012-04-01 12:48:26 PDT
Comment on attachment 135011 [details]
Patch for landing

Rejecting attachment 135011 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
geLog
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebCore/ChangeLog.rej
patching file Source/WebCore/dom/Document.cpp
patching file Source/WebCore/dom/SecurityContext.cpp
patching file Source/WebCore/dom/SecurityContext.h
patching file Source/WebCore/html/HTMLIFrameElement.cpp
patching file Source/WebCore/html/HTMLIFrameElement.h

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/12306374
Comment 8 Eric Seidel (no email) 2012-04-01 17:45:08 PDT
Will land this differently when merging the branch into trunk.