New issue
Advanced search Search tips

Issue 897971 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

2 tests in MimeHandlerViewCrossProcessTest are flaky on Win 7 dbg

Project Member Reported by bsep@chromium.org, Oct 22

Issue description

Seeing flakes on the Win 7 dbg bot in MimeHandlerViewCrossProcessTest.NavigationRaceFromCrossProcessRenderer/1
 and MimeHandlerViewCrossProcessTest.EmbedWithInitialCrossOriginFrame/1. I will disable the tests.

Example failures of NavigationRaceFromCrossProcessRenderer/1:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20Tests%20%28dbg%29%281%29/72448
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20Tests%20%28dbg%29%281%29/72436

Example failure of EmbedWithInitialCrossOriginFrame/1:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20Tests%20%28dbg%29%281%29/72437

 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 23

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

commit a7963a420ef6eb6ff6f79bf1dc031dedddd7e333
Author: Bret Sepulveda <bsep@chromium.org>
Date: Tue Oct 23 00:22:42 2018

Disable flaky tests in MimeHandlerViewCrossProcessTest.

Disables NavigationRaceFromCrossProcessRenderer and
EmbedWithInitialCrossOriginFrame. They are flaky on Windows 7 debug.

TBR=ekaramad@chromium.org

Bug: 897971
Change-Id: I3616355526307d4420c4176a0b77d17ffac7705e
Reviewed-on: https://chromium-review.googlesource.com/c/1295184
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601795}
[modify] https://crrev.com/a7963a420ef6eb6ff6f79bf1dc031dedddd7e333/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc

Cc: kenrb@chromium.org ekaramad@chromium.org alex...@chromium.org wjmaclean@chromium.org
 Issue 898029  has been merged into this issue.
Components: -Blink Platform>Apps>BrowserTag
Status: Started (was: Assigned)
I see a few remaining issues not properly addressed in CL 1240635 (support for proxies in attach WebContents).

Context:
The FrameNavigationHelper fires a navigation to about:blank and waits for the first finished navigation to about:blank. Then immediately attaches WebContentses (we currently need a frame same process with its parent for attach to happen). Meanwhile, to avoid interference from other navigations for the same FrameTreeNode, throttles any forthcoming navigation which is not to about:blank.

There is a couple of problems here:

1- First off, to attach right after DidFinishNavigation does not seem safe. This will end up destroying the navigation_handle->GetRenderFrameHost() (after calling AttachToOuterWebContentsFrame). An observer such as ContentPasswordManagerDriverFactory is relying on NavigationHandle->GetRenderFrameHost() not been deleted when a navigation commits (note that RFH has private dtor so I think causing a deletion through non-navigation path is possibly a bug).

2- We could have multiple navigations to about:blank which might have started already so we commit multiple times to about:blank for the same frame perhaps. This is just a guess but I believe is possible because the navigations here are from cross-origin to same-origin about:blank and do not seem to be happening synchronously. The scenario I am thinking is when we set 'object.data = "pdf"' and then repeatedly call object.contentDocument.defaultView.location.href = "about:blank". So many about:blank navigations are queued up.

I am working on this design to see if the issue can be resolved without changes to the content API AttachToOuterWebContentsFrame. But I think the comment above the method declaration should point out that the |outer_render_frame_host| will be destroyed soon (synchronously?).
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 4

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

commit dd9a414abfcc248c94dca646d85741cf7c7d956e
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Tue Dec 04 20:38:20 2018

WebContents should attach after navigations commit

To accommodate attaching MimeHandlerViewGuest though a proxy, the
FrameTreeNode is first navigated to about:blank. The current logic in
FrameNavigationHelper waits for the first commit to about:blank and
then continues the attach process. This current behavior has some
problems which this CL tries to resolve.

1- The helper class listens to WebContentsObserver::DidFinishNavigation
to find out when the navigation in the plugin frame commits; then
immediately attaches to the outer WebContents. This is incorrect as
attaching will synchronously destroy the RenderFrameHost which causes
issues with other observers of the navigations in that frame
(NavigationHandle::GetRenderFrameHost() becomes invalid.

2- There could be many queued up navigations to about:blank and some
of them will commit after attach which causes the same problem as 1).

To address issue 1), this CL will post task the attach to give all the
observers the chance to observe the committed navigation. To address
 issue 2 ) new APIs are introduced to content layer which help to reset
a frame's state (navigation requests and loading state) prior to
attaching.

The CL will also enable a test which was disabled due to crashes caused
by the mentioned problems above.

Bug: 897971, 659750
Change-Id: Ie26a7ca644dcbdb1b18e5a659119ea2f46719c0a
Reviewed-on: https://chromium-review.googlesource.com/c/1298354
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613683}
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/chrome/test/data/extensions/api_test/mime_handler_view/test_page.html
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/public/browser/render_frame_host.h
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/public/browser/web_contents.h
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/extensions/browser/guest_view/extensions_guest_view_message_filter.cc
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/extensions/browser/guest_view/extensions_guest_view_message_filter.h
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc

Sign in to add a comment