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

Issue 601581 link

Starred by 1 user
Status: Verified
Owner:
█ r...@opera.com
NOT IN USE
Closed: Apr 2016
Cc:
wfh@chromium.org
creis@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug-Regression

Blocking:
issue 477150



Sign in to add a comment

m_persistedPluginWidget->isFrameView() assert fails in --site-per-process mode

Project Member Reported by lukasza@chromium.org, Apr 7 2016 
Site Isolation FYI bot has caught a regression in https://build.chromium.org/p/chromium.fyi/builders/Site%20Isolation%20Linux/builds/8688


Repro steps:


1. Build with DCHECKs enabled

$ cat out/gn/args.gn 
# Build arguments go here. Examples:
#   is_component_build = true
#   is_debug = false
# See "gn args <out_dir> --list" for available build arguments.
dcheck_always_on = true
is_component_build = true
is_debug = false
use_goma = true


2. Run layout tests with --site-per-process

third_party/WebKit/Tools/Scripts/run-webkit-tests -t gn -v --additional-drt-flag=--site-per-process --additional-drt-flag=--no-sandbox http/tests/security/xss-DENIED-object-element.html


Expected behavior: test passes

Actual behavior: test hits an assert and crashes

Comment 1 by lukasza@chromium.org, Apr 7 2016 

STDERR: ASSERTION FAILED: m_persistedPluginWidget->isFrameView()
STDERR: ../../third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp(95) : void blink::HTMLPlugInElement::setPersistedPluginWidget(blink::Widget *)
STDERR: 1   0x7f9fde7901c5 blink::HTMLObjectElement::parseAttribute(blink::QualifiedName const&, WTF::AtomicString const&, WTF::AtomicString const&)
STDERR: 2   0x7f9fde629c4f blink::Element::attributeChanged(blink::QualifiedName const&, WTF::AtomicString const&, WTF::AtomicString const&, blink::Element::AttributeModificationReason)
STDERR: 3   0x7f9fde63469f blink::Element::didModifyAttribute(blink::QualifiedName const&, WTF::AtomicString const&, WTF::AtomicString const&)
STDERR: 4   0x7f9fde624a74 blink::Element::setAttribute(blink::QualifiedName const&, WTF::AtomicString const&)

Comment 2 by dcheng@chromium.org, Apr 7 2016

Cc: r...@opera.com
Almost certainly https://codereview.chromium.org/1866153002.

Comment 3 by lukasza@chromium.org, Apr 7 2016

Cc: -r...@opera.com
Labels: -Pri-3 Pri-2
Owner: r...@opera.com
Confirmed - this has been caused by https://codereview.chromium.org/1866153002

Comment 4 by lukasza@chromium.org, Apr 7 2016

Summary: m_persistedPluginWidget->isFrameView() assert fails in --site-per-process mode (was: http/tests/security/xss-DENIED-object-element.html hits an assert in --site-per-process mode)

Comment 5 by r...@opera.com, Apr 7 2016 

The assert is probably not correct. It assumes the stored widget is either PluginView or FrameView. My guess is that we're hitting a RemoteFrameView?
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 7 2016 

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

commit 315cc7123ae79a389900c4c28856577fd042709a
Author: lukasza <lukasza@chromium.org>
Date: Thu Apr 07 22:33:42 2016

Add test expectations for recent regressions caught by Site Isolation FYI bot.

BUG= 601584 ,  601581 
NOTRY=true

Review URL: https://codereview.chromium.org/1867003005

Cr-Commit-Position: refs/heads/master@{#385904}

[modify] https://crrev.com/315cc7123ae79a389900c4c28856577fd042709a/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Comment 7 by lukasza@chromium.org, Apr 7 2016 

RE: My guess is that we're hitting a RemoteFrameView?

No - |widget| is null in the repro.

Comment 8 by r...@opera.com, Apr 7 2016

Status: Started (was: Untriaged)
Not the parameter, the persisted member, which is non-null.
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 8 2016 

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

commit b9d69b613d2dbfe770e1b35447c3f957cc515e33
Author: rune <rune@opera.com>
Date: Fri Apr 08 06:14:20 2016

Plugin element widget may be a RemoteFrameView.

Corrected ASSERT and re-enabled test.

Removed ENABLE(OILPAN) ifdef in the neighborhood since its removal is
in progress.

R=dcheng@chromium.org,lukasza@chromium.org
BUG= 601581 

Review URL: https://codereview.chromium.org/1872653002

Cr-Commit-Position: refs/heads/master@{#386005}

[modify] https://crrev.com/b9d69b613d2dbfe770e1b35447c3f957cc515e33/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
[modify] https://crrev.com/b9d69b613d2dbfe770e1b35447c3f957cc515e33/third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp

Comment 10 by r...@opera.com, Apr 8 2016

Labels: OS-All
Status: Fixed (was: Started)

Comment 11 by lukasza@chromium.org, Apr 8 2016

Status: Verified (was: Fixed)
Thanks for fixing this.  The CL from #c9 was first present in https://build.chromium.org/p/chromium.fyi/builders/Site%20Isolation%20Linux/builds/8706 and the bot doesn't hit the failure anymore.

Comment 12 by sshru...@google.com, May 18 2016

Labels: Test-Layout
►

Sign in to add a comment