New issue
Advanced search Search tips

Issue 916687 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

DCHECK when using old Cast dialog

Project Member Reported by dfried@chromium.org, Dec 19

Issue description

What steps will reproduce the problem?
(1) Launch Chrome in debug
(2) Hide all extension icons
(3) Change the Cast flag to use the old (non-views) dialog (#views-cast-dialog)
(4) Navigate to New Tab Page
(5) Select Cast... from the app menu

What is the expected result?
Cast dialog opens.

What happens instead?
DCHECK in toolbar_actions_bar.cc:223:

  DCHECK_EQ(model_->toolbar_items().size(), toolbar_actions_.size());

 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 21

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

commit f93bb3edd577ff34d61ebbb2b9b666a0f957355b
Author: Dana Fried <dfried@chromium.org>
Date: Fri Dec 21 00:18:55 2018

Don't try to lay out extensions bar in menu during add operation.

This fixes a DCHECK caused by a race condition between listeners.
It is possible to try to resize a ToolbarActionsBar which has not yet
been notified that an action was added; the inconsistency resulted in
an immediate layout, which resulted in a DCHECK failure in
toolbar_actions_bar.cc:223:

  DCHECK_EQ(model_->toolbar_items().size(), toolbar_actions_.size());

This bug occurs deterministically when launching the old Cast dialog,
which adds its icon to the browser actions container rather than as a
first-class toolbar icon as the new Cast dialog does.

Now we only lay out the view after all of the callbacks take place, so
the containers all have the proper number of things in them. We remove
the callback on resize started, since the only time this can affect
things is if the actions bar in the toolbar is resizing smaller while
the menu is open, which should not happen.

We continue to lay out after a drag operation, because we need to
immediately resize the view if the number of lines of icons changes.

Bug:  916687 
Change-Id: Id22c6f1697ace1cddc8813016c02a488fb5134bb
Reviewed-on: https://chromium-review.googlesource.com/c/1384929
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618398}
[modify] https://crrev.com/f93bb3edd577ff34d61ebbb2b9b666a0f957355b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
[modify] https://crrev.com/f93bb3edd577ff34d61ebbb2b9b666a0f957355b/chrome/browser/ui/toolbar/toolbar_actions_bar_observer.h
[modify] https://crrev.com/f93bb3edd577ff34d61ebbb2b9b666a0f957355b/chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.cc
[modify] https://crrev.com/f93bb3edd577ff34d61ebbb2b9b666a0f957355b/chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.h

Status: Fixed (was: Started)

Sign in to add a comment