Bug 149874 - Generated frame tree names should be kept reasonably long.
Summary: Generated frame tree names should be kept reasonably long.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords: Performance
Depends on:
Blocks:
 
Reported: 2015-10-07 05:47 PDT by Andreas Kling
Modified: 2015-10-08 15:43 PDT (History)
5 users (show)

See Also:


Attachments
Patch for EWS (177.51 KB, patch)
2015-10-07 06:06 PDT, Andreas Kling
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-mavericks (321.02 KB, application/zip)
2015-10-07 06:39 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-mavericks-wk2 (343.09 KB, application/zip)
2015-10-07 06:43 PDT, Build Bot
no flags Details
Patch for EWS 2 (7.36 KB, patch)
2015-10-07 07:42 PDT, Andreas Kling
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (729.68 KB, application/zip)
2015-10-07 08:26 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews101 for mac-mavericks (698.50 KB, application/zip)
2015-10-07 08:39 PDT, Build Bot
no flags Details
Patch (12.81 KB, patch)
2015-10-07 13:13 PDT, Andreas Kling
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-mavericks (694.33 KB, application/zip)
2015-10-07 13:39 PDT, Build Bot
no flags Details
Patch (13.53 KB, patch)
2015-10-07 13:40 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2015-10-07 05:47:49 PDT
Some ad scripts are wreaking havoc on WebKit sessions states by setting iframe "name" attributes to >100kB of JavaScript.
These frames then get some subframes inserted, and we generate absolutely filthy unique names for these subframes.
Comment 1 Andreas Kling 2015-10-07 06:06:25 PDT
Created attachment 262597 [details]
Patch for EWS

Let's see what EWS thinks of this.
Comment 2 Build Bot 2015-10-07 06:39:50 PDT
Comment on attachment 262597 [details]
Patch for EWS

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

Number of test failures exceeded the failure limit.
Comment 3 Build Bot 2015-10-07 06:39:53 PDT
Created attachment 262600 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 4 Build Bot 2015-10-07 06:43:16 PDT
Comment on attachment 262597 [details]
Patch for EWS

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

Number of test failures exceeded the failure limit.
Comment 5 Build Bot 2015-10-07 06:43:18 PDT
Created attachment 262601 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 6 Andreas Kling 2015-10-07 07:42:40 PDT
Created attachment 262605 [details]
Patch for EWS 2

Oh wow. Let's try to avoid a rebaselining nightmare by staying more faithful to the original format.
Comment 7 Build Bot 2015-10-07 08:26:14 PDT
Comment on attachment 262605 [details]
Patch for EWS 2

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

New failing tests:
http/tests/security/dataURL/xss-DENIED-to-data-url-sub-frame-2-level.html
http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-to-javscript-url.html
http/tests/security/dataURL/xss-DENIED-from-data-url-sub-frame-2-level.html
fast/forms/form-and-frame-interaction-retains-values.html
http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url.html
http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level.html
http/tests/navigation/image-load-in-subframe-unload-handler.html
Comment 8 Build Bot 2015-10-07 08:26:17 PDT
Created attachment 262606 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 9 Build Bot 2015-10-07 08:39:34 PDT
Comment on attachment 262605 [details]
Patch for EWS 2

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

New failing tests:
http/tests/security/dataURL/xss-DENIED-to-data-url-sub-frame-2-level.html
http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-to-javscript-url.html
http/tests/security/dataURL/xss-DENIED-from-data-url-sub-frame-2-level.html
fast/forms/form-and-frame-interaction-retains-values.html
http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url.html
http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level.html
fast/frames/long-names-in-nested-subframes.html
http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-sub-frame-2-level.html
http/tests/navigation/image-load-in-subframe-unload-handler.html
Comment 10 Build Bot 2015-10-07 08:39:36 PDT
Created attachment 262609 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 11 Andreas Kling 2015-10-07 13:13:02 PDT
Created attachment 262629 [details]
Patch
Comment 12 Build Bot 2015-10-07 13:39:51 PDT
Comment on attachment 262629 [details]
Patch

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

New failing tests:
http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-sub-frame-2-level.html
Comment 13 Build Bot 2015-10-07 13:39:54 PDT
Created attachment 262635 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 14 Andreas Kling 2015-10-07 13:40:09 PDT
Created attachment 262636 [details]
Patch
Comment 15 Darin Adler 2015-10-08 09:33:17 PDT
Comment on attachment 262636 [details]
Patch

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

> Source/WebCore/page/FrameTree.cpp:94
> +    unsigned index = 0;
> +    for (Frame* frame = m_parent->tree().firstChild(); frame; frame = frame->tree().nextSibling()) {
> +        if (&frame->tree() == this)
> +            return index;
> +        ++index;
> +    }

I would have walked backwards from this looking for null instead. Not sure why.

> Source/WebCore/page/FrameTree.cpp:178
> +            name.appendNumber(frame->tree().indexInParent());

Is there any N^2 problem with walking the frames to compute the index?
Comment 16 Andreas Kling 2015-10-08 14:57:37 PDT
(In reply to comment #15)
> Comment on attachment 262636 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=262636&action=review
> 
> > Source/WebCore/page/FrameTree.cpp:94
> > +    unsigned index = 0;
> > +    for (Frame* frame = m_parent->tree().firstChild(); frame; frame = frame->tree().nextSibling()) {
> > +        if (&frame->tree() == this)
> > +            return index;
> > +        ++index;
> > +    }
> 
> I would have walked backwards from this looking for null instead. Not sure
> why.

k :)

> > Source/WebCore/page/FrameTree.cpp:178
> > +            name.appendNumber(frame->tree().indexInParent());
> 
> Is there any N^2 problem with walking the frames to compute the index?

I doubt there is a problem in practice, since trees rarely have more than 1 or 2 frames  per "level of ancestry."

If you think it could be a real problem, we could track in-parent indices on FrameTree. FrameTree only supports append and remove operations for children, so it wouldn't be too difficult to track.
Comment 17 WebKit Commit Bot 2015-10-08 15:43:50 PDT
Comment on attachment 262636 [details]
Patch

Clearing flags on attachment: 262636

Committed r190752: <http://trac.webkit.org/changeset/190752>
Comment 18 WebKit Commit Bot 2015-10-08 15:43:57 PDT
All reviewed patches have been landed.  Closing bug.