Bug 82826 - Add security checks for iframe seamless
Summary: Add security checks for iframe seamless
Status: RESOLVED LATER
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks: 45950
  Show dependency treegraph
 
Reported: 2012-03-31 01:19 PDT by Eric Seidel (no email)
Modified: 2012-04-01 17:45 PDT (History)
3 users (show)

See Also:


Attachments
Patch (5.86 KB, patch)
2012-03-31 01:20 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (6.01 KB, patch)
2012-04-01 12:46 PDT, Eric Seidel (no email)
webkit.review.bot: commit-queue-
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) 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.