New issue
Advanced search Search tips

Issue 673771 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 632361



Sign in to add a comment

[PlzNavigate] "RendererCrash" tests pass for the wrong reasons

Project Member Reported by droger@chromium.org, Dec 13 2016

Issue description

The browser_tests:
PrerenderBrowserTest.PrerenderRendererCrash
NoStatePrefetchBrowserTest.RendererCrash

are supposed to check that crashing the prerender is recorded as FINAL_STATUS_RENDERER_CRASHED.

They do this by launching a prerender of chrome://crash.

With PlzNavigate though, no renderer is created for the main navigation, and nothing crashes.
It returns a FINAL_STATUS_UNSUPPORTED_SCHEME status (which is expected because we don't support prerendering chrome:// urls).

The tests pass currently though, because of other bugs and incomplete support of PlzNavigate in prerendering code.

Adding correct support for PlzNavigate is surfacing this.
 

Comment 1 by pasko@chromium.org, Dec 14 2016

Cc: clamy@chromium.org
Labels: -Pri-3 OS-All Pri-2
Status: Assigned (was: Untriaged)
nice find! +clamy FYI

Comment 2 by pasko@chromium.org, Dec 14 2016

Blocking: 632361

Comment 3 by clamy@chromium.org, Dec 14 2016

Normally we special case the debug urls like chrome://crash and have them commit immediately, without being sent to the network stack.

Comment 4 by pasko@chromium.org, Dec 15 2016

For future reference, in the fix [1] droger@ says:
> The reason why it was failing the test is a bit complex. Here are some
> explanations for posterity:
> 
> This was making the test fail because it would cause
> FINAL_STATUS_UNSUPPORTED_SCHEME when requesting chrome://crash instead of
> letting it go through and crashing the rendrerer.
> 
> This was only happening with plz navigate because of an ordering issue: with plz
> navigate this code is called before the renderer is invoked, whereas with the
> old flow the renderer is invoked first (and crashes before running this code).
> 
> Note that chrome://crash is able to go through in the test, because the check
> for unsupported URLs in PrerenderContents is specifically disabled by
> TestPrerenderContents.
> So in real life, prerendering chrome://crash would not crash the prerenderer, it
> only can do so in the test.

[1] https://codereview.chromium.org/2576443002/
Status: Fixed (was: Assigned)

Sign in to add a comment