New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 755328 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on:
issue 755272



Sign in to add a comment

Bunch of WebView browser_tests fail with --mash

Project Member Reported by sky@chromium.org, Aug 14 2017

Issue description

Here's the list.

WebViewTests/WebViewDPITest.Shim_TestAutosizeBeforeNavigation/0
WebViewTests/WebViewDPITest.Shim_TestAutosizeBeforeNavigation/1
WebViewTests/WebViewDPITest.Shim_TestAutosizeHeight/0
WebViewTests/WebViewDPITest.Shim_TestAutosizeHeight/1
WebViewTests/WebViewDPITest.Shim_TestAutosizeRemoveAttributes/0
WebViewTests/WebViewDPITest.Shim_TestAutosizeRemoveAttributes/1
WebViewTests/WebViewFocusTest.TouchFocusesEmbedder/0
WebViewTests/WebViewSizeTest.AutoSize/0
WebViewTests/WebViewSizeTest.AutoSize/1
WebViewTests/WebViewSizeTest.Shim_TestAutosizeBeforeNavigation/0
WebViewTests/WebViewSizeTest.Shim_TestAutosizeBeforeNavigation/1
WebViewTests/WebViewSizeTest.Shim_TestAutosizeHeight/0
WebViewTests/WebViewSizeTest.Shim_TestAutosizeHeight/1
WebViewTests/WebViewSizeTest.Shim_TestAutosizeRemoveAttributes/0
WebViewTests/WebViewSizeTest.Shim_TestAutosizeRemoveAttributes/1
WebViewTests/WebViewSizeTest.Shim_TestResizeEvents/0
WebViewTests/WebViewSizeTest.Shim_TestResizeEvents/1
WebViewTests/WebViewSizeTest.Shim_TestResizeWebviewResizesContent/0
WebViewTests/WebViewSizeTest.Shim_TestResizeWebviewResizesContent/1
WebViewTests/WebViewSizeTest.Shim_TestResizeWebviewWithDisplayNoneResizesContent/0
WebViewTests/WebViewSizeTest.Shim_TestResizeWebviewWithDisplayNoneResizesContent/1
WebViewTests/WebViewTest.InterstitialPageFocusedWidget/1


I suspect these are failing for the same reason, if not, we should split out accordingly.

At least some (if not all) of these test have embedded content. These may be failing for the same reason as 755272. At a minimum we should wait until that is fixed before reevaluating.
 

Comment 1 by sky@chromium.org, Aug 18 2017

Add NaClExtensionTest.MainFrameIsRemote to that list.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 9 2017

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

commit ecf6700cd530a95eb5d09b9c4919d520ebc0e5a5
Author: Scott Violet <sky@chromium.org>
Date: Mon Oct 09 22:12:24 2017

chromeos: enable some WebView tests

These now pass because of OOPIF working in mus. Need to evaluate why
remaining ones fail.

BUG=755328
TEST=test only change

Change-Id: I02dd9a7ad1a46cdfc8475605cb9eeea5ed0619a0
Reviewed-on: https://chromium-review.googlesource.com/706590
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507505}
[modify] https://crrev.com/ecf6700cd530a95eb5d09b9c4919d520ebc0e5a5/testing/buildbot/filters/mus.browser_tests.filter

Comment 3 by sky@chromium.org, Oct 11 2017

Owner: fsam...@chromium.org
Status: Assigned (was: Untriaged)
This is now down to:

-NaClExtensionTest.MainFrameIsRemote
-WebViewTests/WebViewDPITest.Shim_TestAutosizeHeight/0
-WebViewTests/WebViewDPITest.Shim_TestAutosizeHeight/1
-WebViewTests/WebViewDPITest.Shim_TestAutosizeRemoveAttributes/0
-WebViewTests/WebViewDPITest.Shim_TestAutosizeRemoveAttributes/1
-WebViewTests/WebViewFocusTest.TouchFocusesEmbedder/0
-WebViewTests/WebViewSizeTest.AutoSize/0
-WebViewTests/WebViewSizeTest.Shim_TestAutosizeHeight/0
-WebViewTests/WebViewSizeTest.Shim_TestAutosizeHeight/1
-WebViewTests/WebViewSizeTest.Shim_TestAutosizeRemoveAttributes/0
-WebViewTests/WebViewSizeTest.Shim_TestAutosizeRemoveAttributes/1
-WebViewTests/WebViewTest.InterstitialPageFocusedWidget/1

These all may be related. I looked at WebViewTests/WebViewDPITest.Shim_TestAutosizeHeight/1 . AFAICT we never end up triggering processing a proper layout of the page after the auto-resize. LocalFrameView does schedule one (LocalFrameView::ScheduleRelayout, which calls to PageAnimator::ScheduleVisualUpdate, and SendCommitRequestToImplThreadIfNeeded), but layout processing happens during a main-frame, and this code only seems to trigger one main-frame and no more. An initial BeginMainFrame() is called, and subsequently ProxyMain::SendCommitRequestToImplThreadIfNeeded() is called, which makes its way to SchedulerStateMachine::SetNeedsBeginMainFrame(), but the Scheduler loop never ends up in ACTION_SEND_BEGIN_MAIN_FRAME. The Scheduler loop cycles through something like: ACTION_DRAW_IF_POSSIBLE, ACTION_PREPARE_TILES, ACTION_NONE. This appears to be because commits are deferred and never enabled (SchedulerStateMachine::CouldSendBeginMainFrame() returns false because defer_commits_ is true).

The deferring seems to be forced to true because RenderWidget::AutoResizeCompositor reset the LocalSurfaceId of the LayerTreeHost. That implicitly sets defer_commits_ to true. It doesn't seem as though the LocalSurfaceId of the RenderWidget is ever set to a valid value after AutoResizeCompositor, so that deferred remains true. This may be because the browser is waiting for ack that is never sent.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 12 2017

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

commit c194246059ac6684fd5a8fdf6abab6ebb5646fb4
Author: Scott Violet <sky@chromium.org>
Date: Thu Oct 12 22:45:37 2017

chromeos: make mus/mash send frame rects on resize

Based on discussion it seems we should send the frame rects to the
browser even in mus/mash.

BUG=755328
TEST=none

Change-Id: I944e94a655c23f6e6edcfdd51a5dffb933de4716
Reviewed-on: https://chromium-review.googlesource.com/713440
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508512}
[modify] https://crrev.com/c194246059ac6684fd5a8fdf6abab6ebb5646fb4/content/renderer/render_frame_proxy.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 25 2017

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

commit 13e9b1509501a00090df8599eb02fae4e8e96262
Author: Jonathan <jonross@chromium.org>
Date: Wed Oct 25 14:54:33 2017

Disabling flaking mus_browser_tests

WebView related tests are flaking. With AutoSize timing out, and
PDFExtensionTest using an unsupported util.

TBR=sky@chromium.org

Bug: 755328,  763452 
Change-Id: I0c9388e880b8d0923e7df05cbfac0d7a7ba9e186
Reviewed-on: https://chromium-review.googlesource.com/737578
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511457}
[modify] https://crrev.com/13e9b1509501a00090df8599eb02fae4e8e96262/testing/buildbot/filters/mojo.fyi.mus.browser_tests.filter
[modify] https://crrev.com/13e9b1509501a00090df8599eb02fae4e8e96262/testing/buildbot/filters/mus.browser_tests.filter

Comment 6 by sky@chromium.org, Nov 6 2017

Blocking: -740655 731255
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 10 2017

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

commit f81cd1525ca35ea3185e28f2bb1ef149d8e9e4ae
Author: Scott Violet <sky@chromium.org>
Date: Fri Nov 10 01:35:38 2017

chromeos: enables more tests for browser_tests --mus

This makes mojo.fyi.mus.browser_tests.filter match that of the one
used on the main waterfall (mus.browser_tests.filter) and enables some
WebView tests that now pass locally.

BUG=755328
TEST=test only change
TBR=msw@chromium.org

Change-Id: I65addd8e67d11d392a762f55ff110019bf88a6ed
Reviewed-on: https://chromium-review.googlesource.com/762102
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515404}
[modify] https://crrev.com/f81cd1525ca35ea3185e28f2bb1ef149d8e9e4ae/testing/buildbot/filters/mojo.fyi.mus.browser_tests.filter

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 13 2017

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

commit 7d42428d0212aa3a337168c95e0f4d0a6cd2e974
Author: Scott Violet <sky@chromium.org>
Date: Mon Nov 13 22:44:31 2017

chromeos: update browser_tests --mus from fyi filter file

The FYI bot ran without any failures in browser_tests for the weekend.

BUG= 755272 ,755328
TEST=test only change

Change-Id: I104e75be6c0f6edf767cb1e67eafb4514725d1c2
Reviewed-on: https://chromium-review.googlesource.com/767015
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516085}
[modify] https://crrev.com/7d42428d0212aa3a337168c95e0f4d0a6cd2e974/testing/buildbot/filters/mus.browser_tests.filter

Cc: jonr...@chromium.org
InterstitialPageRouteEvents has actually been passing incorrectly.

It attempts to verify that the guest view has an appropriately connected input event router.

This is passing because the browser side RenderWidgetHostViewChildFrame is creating its own FrameSinkId and using it locally. It sends that id to the RenderFrameProxy, but that is ignored in --mus. So there isn't a proper connection, but the subsequent lookup in the test passes.

The RenderFrameProxy is creating the actual FrameSinkId for the embedded content. A patch I'm working on sends this information back to the RWHVChildFrame. (https://chromium-review.googlesource.com/c/chromium/src/+/748772)

Due to this the test starts failing as it finds that the input routing is not available. This properly reflects the state of using a guest view in --mus, as input doesn't actually work right now.

I'll disable the test when landing the above.

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 15 2017

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

commit 9e66f6a2648e6dd60e292806c011a45d179b76d5
Author: Jonathan <jonross@chromium.org>
Date: Wed Nov 15 20:08:53 2017

Have Mus Embedded Frames Tell Child Frame of new FrameSinkId

In --mus the embedding of child guest frames happens in the viz service.

This introduces a new frame message which allows RenderFrameProxy to notify the
CrossProcessFrameConnector of a new FrameSinkId. This is then set on the
associated RenderWidgetHostViewChildFrame.

This will unblock being able to unittest guest views.

TBR=sky@chromium.org
TEST= manual testing of chrome --mus with a guest view.

Bug:  763452 , 755328
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Ie5ebbcf82764d01f7205b1a567506981a8b4b609
Reviewed-on: https://chromium-review.googlesource.com/748772
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516801}
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/content/browser/frame_host/cross_process_frame_connector.cc
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/content/browser/frame_host/cross_process_frame_connector.h
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/content/browser/mus_util.h
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/content/browser/renderer_host/input/touch_selection_controller_client_aura_browsertest.cc
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/content/browser/renderer_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/content/browser/renderer_host/render_widget_host_view_child_frame_browsertest.cc
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/content/common/frame_messages.h
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/content/renderer/render_frame_proxy.cc
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/content/test/content_browser_test_utils_internal.cc
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/content/test/content_browser_test_utils_internal.h
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/testing/buildbot/filters/mojo.fyi.mus.browser_tests.filter
[modify] https://crrev.com/9e66f6a2648e6dd60e292806c011a45d179b76d5/testing/buildbot/filters/mus.browser_tests.filter

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 17 2017

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

commit 4312e7962db39d22e2a6416aa42da13313d0bc48
Author: Scott Violet <sky@chromium.org>
Date: Fri Nov 17 01:38:50 2017

Makes ClientLayerTreeFrameSink call DidLoseLayerTreeFrameSink on connection error

This way reconnection can be attempted.

BUG=755328
TEST=covered by test

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I1c07ef7bb04ca048146392a2f33de5b8ea83ee35
Reviewed-on: https://chromium-review.googlesource.com/775044
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517236}
[modify] https://crrev.com/4312e7962db39d22e2a6416aa42da13313d0bc48/cc/test/fake_layer_tree_frame_sink_client.cc
[modify] https://crrev.com/4312e7962db39d22e2a6416aa42da13313d0bc48/components/viz/BUILD.gn
[modify] https://crrev.com/4312e7962db39d22e2a6416aa42da13313d0bc48/components/viz/client/BUILD.gn
[modify] https://crrev.com/4312e7962db39d22e2a6416aa42da13313d0bc48/components/viz/client/DEPS
[modify] https://crrev.com/4312e7962db39d22e2a6416aa42da13313d0bc48/components/viz/client/client_layer_tree_frame_sink.cc
[modify] https://crrev.com/4312e7962db39d22e2a6416aa42da13313d0bc48/components/viz/client/client_layer_tree_frame_sink.h
[add] https://crrev.com/4312e7962db39d22e2a6416aa42da13313d0bc48/components/viz/client/client_layer_tree_frame_sink_unittest.cc
[modify] https://crrev.com/4312e7962db39d22e2a6416aa42da13313d0bc48/components/viz/client/local_surface_id_provider.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Nov 17 2017

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

commit 4876a33127ef05bdf322ab84448c292b29ae0bb4
Author: Scott Violet <sky@chromium.org>
Date: Fri Nov 17 22:39:46 2017

chromeos (--mus): wires recreation of browser plugins correctly for mus

When a browser plugin is recreated we destroy and create a new
BrowserPlugin. This necessitates reembedding the WindowTreeClient
from the renderer rendering the plugin (because the BrowserPlugin ows
the mus window).

To get the reembeding working required changing
RenderWidgetHostViewGuest to initiate embedding when attached, not
init.

As the renderer rendering the plugin gets a new connection to mus it
needed to be updated. A result of this is the connection to viz is
being dropped, which means ClientLayerTreeFrameSink is seeing it's
connection dropped. The connection is reestablished by way of calling
DidLoseLayerTreeFrameSink().

BUG=755328
TEST=covered by test

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I43838b38bedaf75a341262fcba3befaca77d5c87
Reviewed-on: https://chromium-review.googlesource.com/773104
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517593}
[modify] https://crrev.com/4876a33127ef05bdf322ab84448c292b29ae0bb4/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/4876a33127ef05bdf322ab84448c292b29ae0bb4/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/4876a33127ef05bdf322ab84448c292b29ae0bb4/content/browser/frame_host/render_widget_host_view_guest.h
[modify] https://crrev.com/4876a33127ef05bdf322ab84448c292b29ae0bb4/content/renderer/browser_plugin/browser_plugin.cc
[modify] https://crrev.com/4876a33127ef05bdf322ab84448c292b29ae0bb4/content/renderer/mus/mus_embedded_frame.cc
[modify] https://crrev.com/4876a33127ef05bdf322ab84448c292b29ae0bb4/content/renderer/mus/mus_embedded_frame.h
[modify] https://crrev.com/4876a33127ef05bdf322ab84448c292b29ae0bb4/content/renderer/mus/renderer_window_tree_client.cc
[modify] https://crrev.com/4876a33127ef05bdf322ab84448c292b29ae0bb4/testing/buildbot/filters/mojo.fyi.mus.browser_tests.filter
[modify] https://crrev.com/4876a33127ef05bdf322ab84448c292b29ae0bb4/testing/buildbot/filters/mus.browser_tests.filter

Blocking: -731255
Summary: Bunch of WebView browser_tests fail with --mash (was: Bunch of WebView browser_tests fail with --mus)
After decoupling from viz, the browser-tests now work OK with --mus.
Cc: r...@chromium.org
They started failing on a bunch of other bots:

https://ci.chromium.org/buildbot/chromium.clang/CrWinClangLLD64/537
https://ci.chromium.org/buildbot/chromium.clang/ToTWin%28dll%29/667

Did anything change here recently?

https://ci.chromium.org/p/chromium/g/chromium.clang/console is pretty red in part of this bug. Can you look at this soon, or disable the tests?

Comment 15 by sky@chromium.org, Apr 17 2018

Mash is ChromeOS specific. So, the windows changes should not be related.
Project Member

Comment 16 by bugdroid1@chromium.org, Apr 27 2018

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

commit 7a457481ec0869ac5d8379357c80fc1f992827b7
Author: James Cook <jamescook@chromium.org>
Date: Fri Apr 27 18:49:50 2018

cros: Enable WebViewSizeTest in mash_browser_tests

It was originally disabled because it was broken in mus, but it works
now.

Also update the comments for related disabled WebView tests.

Bug: 755328
Change-Id: I888957ed89a0a3fb101b965be1666e11d32ebdb9
Reviewed-on: https://chromium-review.googlesource.com/1029226
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554430}
[modify] https://crrev.com/7a457481ec0869ac5d8379357c80fc1f992827b7/testing/buildbot/filters/mash.browser_tests.filter

Cc: kylec...@chromium.org samans@chromium.org fsam...@chromium.org
Owner: moh...@chromium.org
Reassigning to mohsen@. Please let me know if you have any questions! Thanks!

Sign in to add a comment