Issue metadata
Sign in to add a comment
|
Browser actions don't open correctly |
||||||||||||||||||||||
Issue descriptionChrome Version 66.0.3355.0 (Official Build) canary (64-bit) OS Version: OS X 10.13.3 suspecting r538035 What steps will reproduce the problem? 1. Install an extension with a browser action popup (e.g. https://chrome.google.com/webstore/detail/shruggie/gajlbghllhneagnenoigoacccalfnjmd ) 2. Open its browser action What is the expected result? It should open normally. Window shadow etc. (it doesn't need to animate). What happens instead of that? Opens as a small square, that gets a tiny bit bigger after a second. No content shows. If you're lucky, it stays open and _eventually_ it might show the content. That might depend on the extension triggering a re-layout at the right time though. Jumps to a weird place. Disappears. It's weird. Clicking the button twice also doesn't properly dismiss the browser action, but shows it again.
,
Feb 26 2018
For clicking the button twice: Mac seems to handle focusing on the window a bit differently compared to Windows. On Windows, the click on the button is suppressed and as a result, another show doesn't occur. The normal parent window received active code dismissed the dialog. On Mac, we do the parent window receiving active dismissal, except the button also receives a click, triggering the flow again. For sizing I suspect we're going to have to suppress showing the dialog until the first valid size. The views version of the extension popups has a lot of sizing quirks on Windows as well.
,
Feb 27 2018
Now that I look at Windows a bit more closely (remote), Windows doesn't dismiss on release builds either. Seems like there's an interesting issue in views in general.
,
Feb 27 2018
the button thing might be Issue 459895 (/me hides).
,
Feb 27 2018
Is this indeed a dupe of issue 816853 -- they look identical? If this is behaving bad on Windows as well, that suggests it may be a bug in surface sync in general (which is going to stable on Windows soon)
,
Feb 27 2018
This is made worse by issue 816853 . The intermediate stage of the extension resize seems to last longer. I've got a fix to get rid of the intermediate stage in progress.
,
Feb 27 2018
,
Feb 27 2018
The issue as I understand it is as follows: In RenderWidgetHostImpl::OnResizeOrRepaintACK we post a task to base::ThreadTaskRunnerHandle::Get(). But then during view resize, we pump a nested run loop in RenderWidgetHostImpl::PauseForPendingResizeOrRepaints, and the task runner for that nested run loop is different (it's from ui::WindowResizeHelperMac), so we end up missing that message (or getting it in an unexpected order). Two ideas for fixes: 1. Make RenderWidgetHostImpl::OnResizeOrRepaintACK use the same task runner from ui::WindowResizeHelperMac, so we can get it inside RenderWidgetHostImpl::PauseForPendingResizeOrRepaints. 2. Disable RenderWidgetHostImpl::PauseForPendingResizeOrRepaints during auto-resize. I think that [2] is safer.
,
Feb 27 2018
Issue 816853 has been merged into this issue.
,
Feb 28 2018
,
Feb 28 2018
... and the reason why all ExtensionPopups start out as 25x25 and then suddenly resize to the correct size is because enabling autosizing seems to set an initial size.
void ExtensionViewViews::RenderViewCreated(
content::RenderViewHost* render_view_host) {
extensions::ViewType host_type = host_->extension_host_type();
if (host_type == extensions::VIEW_TYPE_EXTENSION_POPUP) {
host_->render_view_host()->EnableAutoResize(
gfx::Size(ExtensionPopup::kMinWidth, ExtensionPopup::kMinHeight),
gfx::Size(ExtensionPopup::kMaxWidth, ExtensionPopup::kMaxHeight));
}
}
Unsurprisingly...
const int ExtensionPopup::kMinWidth = 25;
const int ExtensionPopup::kMinHeight = 25;
,
Feb 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eb1fe5485b437a580ab205870b4d31640301595b commit eb1fe5485b437a580ab205870b4d31640301595b Author: Christopher Cameron <ccameron@chromium.org> Date: Wed Feb 28 04:06:47 2018 Disable PauseForPendingResizeOrRepaints in AutoResize In RenderWidgetHostImpl::OnResizeOrRepaintACK, we post a task to base::ThreadTaskRunnerHandle::Get(). During resize, in RenderWidgetHostImpl::PauseForPendingResizeOrRepaints, we pump a nested run loop on ui::WindowResizeHelperMac::task_runner, which will not run this message. This ends up causing the message to be received at an unexpected or delayed time. Avoid this by disallowing PauseForPendingResizeOrRepaints for AutoResize. In principle the task could be posted to the WindowResizeHelperMac task runner, but it's hard to be certain that that would not introduce bugs. Bug: 816421 Change-Id: Iaf8bdcc99e4b40d41e2a9590e4436d07c450d403 Reviewed-on: https://chromium-review.googlesource.com/940444 Reviewed-by: Robert Liao <robliao@chromium.org> Commit-Queue: ccameron <ccameron@chromium.org> Cr-Commit-Position: refs/heads/master@{#539701} [modify] https://crrev.com/eb1fe5485b437a580ab205870b4d31640301595b/content/browser/renderer_host/render_widget_host_impl.cc
,
Feb 28 2018
,
Mar 1 2018
Verified the fix on Mac 10.13.1 with Chrome version #66.0.3358.0 as per the comment#0 Attaching screen cast for reference. Observed, after clicking the icon it opened normally with out any square. Hence, the fix is working as expected. Adding the verified labels. Thanks! |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by robliao@chromium.org
, Feb 26 2018