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

Issue 2715933002: TEST: Removing PlatformAppsNotIsolated

Created:
3 years, 9 months ago by arthursonzogni
Modified:
3 years, 9 months ago
Reviewers:
Charlie Reis
CC:
chromium-reviews, lazyboy, clamy, alexmos, nasko, ncarter (slow), site-isolation-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

TEST: Update PlatformAppsNotIsolated With crbug.com/615585 fixed, we won't allow web content inside sandbox pages' iframes, so this test is incorrect. Instead, this CL modifies the test to assert that no OOPIF is created, even for loading the error page. Currently, there is still no error pages. Displaying error pages will be enabled with PlzNavigate from this CL: https://codereview.chromium.org/2655463006/ BUG=612711, 615585

Patch Set 1 #

Patch Set 2 : Modifying the test instead of deleting it. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -10 lines) Patch
M chrome/browser/site_details_browsertest.cc View 1 2 chunks +11 lines, -10 lines 0 comments Download

Messages

Total messages: 13 (10 generated)
arthursonzogni
Hi Charlie, This CL has been done after a discussion here with @lazyboy: https://codereview.chromium.org/2655463006#msg95 Please, ...
3 years, 9 months ago (2017-02-24 13:41:04 UTC) #6
Charlie Reis
[+nick, site-isolation-reviews] I have some concerns about removing this, which I posted in https://codereview.chromium.org/2655463006/#msg114. I ...
3 years, 9 months ago (2017-02-24 22:01:52 UTC) #7
arthursonzogni
3 years, 9 months ago (2017-03-06 14:12:57 UTC) #11
On 2017/02/24 22:01:52, Charlie Reis (OOO til Mar 6) wrote:
> [+nick, site-isolation-reviews]
> 
> I have some concerns about removing this, which I posted in
> https://codereview.chromium.org/2655463006/#msg114.
> 
> I definitely agree that the test is a bit out of date, since we no longer load
> the web iframe in process due to the CSP policy added in r441208.  However, it
> might make sense to continue asserting that no OOPIFs are created, unless we
can
> verify it's safe to load an OOPIF in a different StoragePartition from its
> parent.  We did not design to support that, and I'm worried about what
> assumptions it might break.

Hi Charlie,

We will avoid loading an OOPIF inside a PlatformApp V2 thanks to this patch:
https://codereview.chromium.org/2732883003/

I modified the test to continue asserting that no OOPIFs are created.

Powered by Google App Engine
This is Rietveld 408576698