Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(182)

Issue 1282623002: Revert of Introduce cross_site_iframe_factory.html, and use it in a test (Closed)

Created:
5 years, 4 months ago by nasko
Modified:
5 years, 4 months ago
CC:
asvitkine+watch_chromium.org, chromium-reviews, darin-cc_chromium.org, erikwright+watch_chromium.org, site-isolation-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Introduce cross_site_iframe_factory.html, and use it in a test (patchset #9 id:150001 of https://codereview.chromium.org/1264923002/ ) Reason for revert: Fails on Windows Vista and Mac 10.6 http://build.chromium.org/p/chromium.win/builders/Vista%20Tests%20(1)/builds/58432 http://build.chromium.org/p/chromium.mac/builders/Mac10.6%20Tests/builds/5408 ../../chrome/browser/site_details_browsertest.cc:121: Failure Value of: details->uma()->GetAllSamples( "SiteIsolation.IsolateAllSitesTotalProcessCountEstimate") Expected: has 1 element that is equal to Bucket 11: 1 Actual: { Bucket 10: 1 }, whose element #0 doesn't match Original issue's description: > Introduce cross_site_iframe_factory.html, and use it to test > the site isolation code in MemoryMetricsDetails under chrome/. > > [ See crbug.com/515672 for a screenshot ] > > /cross_site_iframe_factory.html?a(b(c, d)) gives you a simple three-level, > four-site iframe test. The trick it uses, is that the query parameter > is parsed and passed recursively to children, so the first level is: > > <iframe src="b.com:1234/cross_site_iframe_factory.html?b(c(),d())"> > > Which then contains the two leaf iframes: > > <iframe src="c.com:1234/cross_site_iframe_factory.html?c()"> > <iframe src="d.com:1234/cross_site_iframe_factory.html?d()"> > > To to cut down on noise. ".com" is automatically affixed to the hostname > if a TLD isn't present. > > As a convenience, the page supports local preview if loaded via file:// URI. > file:// URIs are technically cross-origin, although --site-per-process doesn't > isolate them today. > > Each site has a random (but consistent) background color as a visual aid. > The page contains some crude layout logic to make sure the outer levels > are big enough to fit the inner levels. > > The parsing logic is in tree_parser_util.js, which has unittests. > > MetricsMemoryDetailsBrowserTest.TestSiteIsolation is a new test > that uses the above facility to creates a bunch of complex iframe > cases, and tests that our process-counting UMA stats are as expected. > > BUG=515672 > > Committed: https://crrev.com/02292e994c9ec62deecec3bec2132500ca68a5de > Cr-Commit-Position: refs/heads/master@{#342447} TBR=dcheng@chromium.org,asvitkine@chromium.org,jam@chromium.org,nick@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=515672 Committed: https://crrev.com/324d28e8e26f11bc88a96181eb029658dbe441c6 Cr-Commit-Position: refs/heads/master@{#342473}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -625 lines) Patch
D chrome/browser/site_details_browsertest.cc View 1 chunk +0 lines, -302 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 chunk +0 lines, -1 line 0 comments Download
D content/test/data/cross_site_iframe_factory.html View 1 chunk +0 lines, -169 lines 0 comments Download
D content/test/data/tree_parser_util.js View 1 chunk +0 lines, -153 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
nasko
Created Revert of Introduce cross_site_iframe_factory.html, and use it in a test
5 years, 4 months ago (2015-08-07 23:15:54 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1282623002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1282623002/1
5 years, 4 months ago (2015-08-07 23:17:45 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 4 months ago (2015-08-07 23:19:57 UTC) #3
commit-bot: I haz the power
5 years, 4 months ago (2015-08-07 23:20:34 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/324d28e8e26f11bc88a96181eb029658dbe441c6
Cr-Commit-Position: refs/heads/master@{#342473}

Powered by Google App Engine
This is Rietveld 408576698