Blink crash on PluginPowerSaver for fixed-position plugin within iframe |
|||||
Issue description1. Use tip of tree and then 'git revert c916fccc772dce7924d8cf1266ba4e71344c37c3'. This will restore the PPS patch that was reverted. 2. Run Chrome with Flash. use the --ppapi-flash-path flag. 3. Visit http://pps-test-a.appspot.com/repro_545039.html. 4. Scroll down using mouse wheel or arrow keys. 5. Observe above crash. repro_545039.html contains: <iframe src="repro_545039_iframe.html"></iframe> (a bunch of filler to enable scrolling) repro_545039_iframe.html contains: <div style="position:fixed;"> <object data="http://pps-test-b.appspot.com/flash.swf"></object> </div> The key part is the position: fixed. Without it, or without the iframe, the crash doesn't happen. If this is any clue, when I add a printf statement to LoadablePluginPlaceholder::OnUnobscuredRectUpdate, I receive: OnUnobscuredRectUpdate: 0,0 292x142 OnUnobscuredRectUpdate: 0,0 292x142 OnUnobscuredRectUpdate: 0,0 292x142 OnUnobscuredRectUpdate: 0,0 292x142 OnUnobscuredRectUpdate: 0,-2 292x142 Received signal 4 ILL_ILLOPN 7ff7c01b3d3b The crash occurs when I get the negative number in the y axis.
,
Mar 3 2016
I can't reproduce this using the steps at ToT on Linux.
,
Mar 3 2016
Bizzare. I just tried and it reproed. More details of build environment: 1. My origin/master is at 6e03065cee6ad249a6b8b14e9f690ee6efb86703. That's Mar 3 11:49. 2. Then I revert c916fccc772dce7924d8cf1266ba4e71344c37c3 on top of that. 3. 'GYP_DEFINES': 'use_goma=1 component=shared_library clang=1', same as you. 4. PPAPI_FLASH_PATH = /mnt/src/chromium/src/third_party/adobe/flash/binaries/ppapi/linux_x64/libpepflashplayer.so This seems to work using the one in /opt/google/chrome too. 5. Navigate to http://pps-test-a.appspot.com/repro_545039.html 6. Scroll down with mousewheel. If it makes any difference, I'm on Cinnamon desktop with compositing on. I have a Z620.
,
Mar 3 2016
,
Mar 3 2016
For some reason third_party/adobe/flash/binaries/ppapi/linux_x64/libpepflashplayer.so doesn't work for me - says permission denied when trying to load the shared library. But when loading the other path, it definitely shows the plugin running with the blue background etc.
,
Mar 3 2016
Oh I see. I had overwritten the settings at some point to turn off PPS. I can now reproduce.
,
Mar 4 2016
Levi and I figured out the bug. The problem is that the resize callback inside of plugin_poster.html is called synchronously during the lifecycle update of the parent frame of the webview plugin, which causes layout to get dirtied on the parent frame, due to the behavior of WebPluginContainerImpl::setNeedsLayout, which is caused by animating the webview plugin for resize. The solution is to let it update the size, but forcibly update the lifecycle of the webview plugin in WebViewPlugin::updateGeometry rather than passing up requests to do it later. All of this also allows us to nuke the timer hack in LoadablePluginPlaceholder. We also found a bug in the geometry code for fixed-position elements. I think this *might* be a primary cause of crbug.com/590856. Going to test that theory first before writing more code.
,
Mar 4 2016
,
Mar 4 2016
,
Mar 4 2016
,
Mar 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/37ca7fe6f5baa7074524c9754010aa3f425cced8 commit 37ca7fe6f5baa7074524c9754010aa3f425cced8 Author: chrishtr <chrishtr@chromium.org> Date: Fri Mar 04 06:32:05 2016 In mapAncestorToLocal, account for frame documents always being fixed-position containers. BUG= 591174 Review URL: https://codereview.chromium.org/1767573002 Cr-Commit-Position: refs/heads/master@{#379229} [modify] https://crrev.com/37ca7fe6f5baa7074524c9754010aa3f425cced8/third_party/WebKit/Source/core/layout/LayoutView.cpp [modify] https://crrev.com/37ca7fe6f5baa7074524c9754010aa3f425cced8/third_party/WebKit/Source/core/layout/MapCoordinatesTest.cpp
,
Mar 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/287a7e65b2b5dd05053c1bda4402e0755ad21bf6 commit 287a7e65b2b5dd05053c1bda4402e0755ad21bf6 Author: chrishtr <chrishtr@chromium.org> Date: Fri Mar 04 20:07:43 2016 Negate just the IsFixed bit. BUG= 591174 Review URL: https://codereview.chromium.org/1764223002 Cr-Commit-Position: refs/heads/master@{#379340} [modify] https://crrev.com/287a7e65b2b5dd05053c1bda4402e0755ad21bf6/third_party/WebKit/Source/core/layout/LayoutView.cpp
,
Mar 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/93e8a2374eae0690244c6c1e8b211d17409e2154 commit 93e8a2374eae0690244c6c1e8b211d17409e2154 Author: chrishtr <chrishtr@chromium.org> Date: Fri Mar 04 20:55:14 2016 Update a webview plugin's WebView's lifecycle immediately after resizing. Remove the delay timer before updating a webview plugin's geometry, since the referenced Blink compositing bug has been fixed. BUG= 591174 Review URL: https://codereview.chromium.org/1764043002 Cr-Commit-Position: refs/heads/master@{#379351} [modify] https://crrev.com/93e8a2374eae0690244c6c1e8b211d17409e2154/components/plugins/renderer/webview_plugin.cc [modify] https://crrev.com/93e8a2374eae0690244c6c1e8b211d17409e2154/components/plugins/renderer/webview_plugin.h [modify] https://crrev.com/93e8a2374eae0690244c6c1e8b211d17409e2154/third_party/WebKit/Source/core/frame/FrameView.cpp
,
Mar 4 2016
Tommy, you can now revert the revert of your patch in loadable_plugin_placeholder.cc.
,
Mar 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3d2ec22c9e7ebd46676e6f3190fc168b8afaae2c commit 3d2ec22c9e7ebd46676e6f3190fc168b8afaae2c Author: tommycli <tommycli@chromium.org> Date: Fri Mar 04 22:46:35 2016 Reland: Plugin Power Saver: Remove Size Recheck hack in plugin placeholders. This is a reland of https://codereview.chromium.org/1491893004/. This requires the fixes in https://codereview.chromium.org/1764043002/. BUG= 591174 , 560590 , 497429 Review URL: https://codereview.chromium.org/1764273002 Cr-Commit-Position: refs/heads/master@{#379381} [modify] https://crrev.com/3d2ec22c9e7ebd46676e6f3190fc168b8afaae2c/components/plugins/renderer/loadable_plugin_placeholder.cc [modify] https://crrev.com/3d2ec22c9e7ebd46676e6f3190fc168b8afaae2c/components/plugins/renderer/loadable_plugin_placeholder.h
,
Mar 4 2016
It's not clear if this has to block Chrome 50, depends on the crash rates. But if needed we could merge it. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by tommycli@chromium.org
, Mar 1 2016