New issue
Advanced search Search tips

Issue 620194 link

Starred by 2 users
Status: Available
Owner: ----
Cc:
lazyboy@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

platform_app's iframe creation can happen after the app's main frame gets deleted, during teardown

Project Member Reported by lazyboy@chromium.org, Jun 15 2016 
This is the underlying cause of the test flakiness in  http://crbug.com/619148 . Since this is not directly related to that test, I'm going to work around that test and track the underlying issue here.

See CL for repro:
https://codereview.chromium.org/2062133005/

In the CL, an app's main frame/window is created through chrome.app.window.create().
The main frame creates a child <iframe> in its window.onload handler,
but the test also tries to shutdown inside chrome.app.window.create()'s
creation callback. This exposes the race:

AFAIU, the child frame creation message FrameHostMsg_CreateChildFrame appears to be processed after the main frame of the app is deleted, so it hits a CHECK in web_contents_observer_sanity_checker.cc,
specifically:
through WebContentsObserverSanityChecker::RenderFrameHostChange(),
AssertRenderFrameExists(new_host->GetParent()) will fail complaining:
"Check failed: render_frame_created_happened..."

Note that this happens regardless of --site-per-process or --isolate-extensions

Locally the test in the CL PlatformAppBrowserTest.AppWindowIframe fails at least once in every 10 runs.
See the CL for more info.

Since nick@ is familiar with web_contents_observer_sanity_checker.cc, I'll assign it to him to start. Nick, would you able to take a look? If you need any other info, let me know.
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 17 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c038c14402bbb9d73aeae6a8642f1595d2fc75f

commit 5c038c14402bbb9d73aeae6a8642f1595d2fc75f
Author: lazyboy <lazyboy@chromium.org>
Date: Fri Jun 17 04:21:05 2016

Add a (disabled) test to demonstrate child frame creation race.

This test demonstrates that browser/ process can encounter a child
frame creation message after its parent/main frame got deleted.

This will hit a check in web_contents_observer_sanity_checker.cc,
specifically:
through WebContentsObserverSanityChecker::RenderFrameHostChange(),
AssertRenderFrameExists(new_host->GetParent()) will fail complaining:
"Check failed: render_frame_created_happened..."

An app's main frame/window is created through chrome.app.window.create().
The main frame creates a child <iframe> in its window.onload handler,
but the test also tries to shutdown inside chrome.app.window.create()'s
creation callback. This exposes the race mentioned above.

BUG=620194
Tests=Without fixing the underlying cause, the test will flake, it
locally fails at least once in every 10 runs. No --site-per-process
or --isolate-extensions flag is necessary:
./out/Debug/browser_tests --gtest_filter=PlatformAppBrowserTest.AppWindowIframe
  --gtest_also_run_disabled_tests

Review-Url: https://codereview.chromium.org/2062133005
Cr-Commit-Position: refs/heads/master@{#400353}

[modify] https://crrev.com/5c038c14402bbb9d73aeae6a8642f1595d2fc75f/chrome/browser/apps/app_browsertest.cc
[add] https://crrev.com/5c038c14402bbb9d73aeae6a8642f1595d2fc75f/chrome/test/data/extensions/platform_apps/app_window_send_message/main.html
[add] https://crrev.com/5c038c14402bbb9d73aeae6a8642f1595d2fc75f/chrome/test/data/extensions/platform_apps/app_window_send_message/main.js
[add] https://crrev.com/5c038c14402bbb9d73aeae6a8642f1595d2fc75f/chrome/test/data/extensions/platform_apps/app_window_send_message/manifest.json
[add] https://crrev.com/5c038c14402bbb9d73aeae6a8642f1595d2fc75f/chrome/test/data/extensions/platform_apps/app_window_send_message/test.js

Comment 2 by nick@chromium.org, Mar 3 2017

Status: Started (was: Assigned)
Fix turns out to be easy.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a77bacfbf5f0e7eb11cb69c8e2c1f3abd0dd87d

commit 7a77bacfbf5f0e7eb11cb69c8e2c1f3abd0dd87d
Author: nick <nick@chromium.org>
Date: Mon Mar 06 22:50:22 2017

Fix PlatformAppBrowserTest.AppWindowIframe.

The problem was that the parent RenderFrameHost was receiving an
OnCreateChildFrameCall after RenderProcessGone had been triggered,
resulting in WebContentsObservers seeing the creation of a RFH with
a nonexistant parent.

WebContentsObserverSanityChecker caught this inconsistency.

The solution is to drop the message when this happens.

BUG=620194
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
TBR=benwells@chromium.org

Review-Url: https://codereview.chromium.org/2729253005
Cr-Commit-Position: refs/heads/master@{#454987}

[modify] https://crrev.com/7a77bacfbf5f0e7eb11cb69c8e2c1f3abd0dd87d/chrome/browser/apps/app_browsertest.cc
[modify] https://crrev.com/7a77bacfbf5f0e7eb11cb69c8e2c1f3abd0dd87d/content/browser/frame_host/render_frame_host_impl.cc

Comment 4 by nick@chromium.org, Sep 26

Owner: ----

Comment 5 by benhenry@google.com, Jan 11

Status: Available (was: Started)
This issue has been marked as started, but has no owner. Making available.
►

Sign in to add a comment