I see this on 67.0.3383. It definitely seems like a regression.

weidongg@, can you see if your recent changes might have caused this?

I will try to test this with a 65 build and if that is fine will start a bisect.

Unfortunately if I try to test locally with --ash-host-window-bounds=1200x800,1000x600, the displays are defaulting to mirror mode and if I disable mirror mode I trigger:

FATAL:display_layout_store.cc(91)] Check failed: list.size() > 1u (1 vs. 1)

It's a DCHECK, so maybe I can just ignore it?

I will also make sure my recent changes are not what is triggering the DCHECK.

Ash developer shortcut to add the secondary display used to add the display in extended mode.

I think my changes did not cause this issue, but I found the possible reason for this. Each display block has a display name inside it, the click or tap often occurs on the display name. (Especially for internal display as its size is often set smaller than external display.) The events is not handled by the display name div [1] and not passed to parent div. So clicking or dragging on the display name triggers nothing. One possible fix is to pass the events to parent div?


[1] https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/device_page/display_layout.html?dr&l=74

Could this be related with the question in the click handler? See https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/device_page/display_layout.js?dr&l=210,211.

Also worth checking if this is something that used to work before changing on-click to on-tap at line https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/device_page/display_layout.html?dr&l=70.

It looks like this predates the on-click -> on-tap change.

weidongg@ - I do think this may have been introduced in https://chromium-review.googlesource.com/767012 or https://chromium-review.googlesource.com/767648. Before that each display was a single div. The name was not a separate element.

This is almost certainly the cause:
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/device_page/drag_behavior.js?dr&q=drag_behavi&sq=package:chromium&l=129

Just confirmed that this works before https://chromium-review.googlesource.com/767012, and that removing the extra name div fixes the problem.

Fix is up: https://chromium-review.googlesource.com/c/chromium/src/+/1014624

Re #6, Yes, you are right, it is caused by my change https://chromium-review.googlesource.com/767012. I change the L73-L78 to:

          <template is="dom-if" if="[[!mirroring]]">
            [[item.name]]
          </template>
          <template is="dom-if" if="[[mirroring]]">
            $i18n{displayMirrorDisplayName}
          </template>

And tested locally, the issue is fixed.

FWIW, dom-if around a text-node is hurting performance. See more context at https://chromium.googlesource.com/chromium/src/+/master/styleguide/web/web.md#Polymer (last bullet point).

Yep. The current code (which wraps a <div> node which is also sub optimal) predated our (my) understanding of dom-if complexity. The CL in comment #7 uses a getText_() function instead.

This affects 66 also so we should try to merge it there if possible.

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

commit 8cbea5ecd76e5ee419a9ca10c3e1566f19db62bb
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Tue Apr 17 19:17:36 2018

Settings > Display: Use a single div

Using a separate name div breaks dragging when the name is dragged
and isn't necessary.

Bug:  833036 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ia9bcc8556018e9dd6d76e0dac6221ecbf85997c7
Reviewed-on: https://chromium-review.googlesource.com/1014624
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Weidong Guo <weidongg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551426}
[modify] https://crrev.com/8cbea5ecd76e5ee419a9ca10c3e1566f19db62bb/chrome/browser/resources/settings/device_page/display_layout.html
[modify] https://crrev.com/8cbea5ecd76e5ee419a9ca10c3e1566f19db62bb/chrome/browser/resources/settings/device_page/display_layout.js

This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Hi, this was initially reported in M66, so it's not a M67 regression.  I'll follow the M66 TPM's lead on merge approval.

Right, the bug was actually introduced all the way back in 64 (!) we just only discovered it recently. Clearly we can't merge it to 64 or even 65, but we should merge this to 66 if possible, and 67 for sure. It is very low risk. It can be worked around, but it's a pretty bad user experience.

Approving merge to M67 Chrome OS.  Please work with josafat@ for M66 review.

ok, let's get in M66 too

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

commit ae9e6368ca3d29f93418e6bc2edf98d1299bf137
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Fri Apr 20 23:13:49 2018

Settings > Display: Use a single div

Using a separate name div breaks dragging when the name is dragged
and isn't necessary.

TBR=stevenjb@chromium.org

(cherry picked from commit 8cbea5ecd76e5ee419a9ca10c3e1566f19db62bb)

Bug:  833036 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ia9bcc8556018e9dd6d76e0dac6221ecbf85997c7
Reviewed-on: https://chromium-review.googlesource.com/1014624
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Weidong Guo <weidongg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#551426}
Reviewed-on: https://chromium-review.googlesource.com/1022850
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#746}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/ae9e6368ca3d29f93418e6bc2edf98d1299bf137/chrome/browser/resources/settings/device_page/display_layout.html
[modify] https://crrev.com/ae9e6368ca3d29f93418e6bc2edf98d1299bf137/chrome/browser/resources/settings/device_page/display_layout.js

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

commit 9b9d2ce47a0f6d9849d8dde3393405d0e17a4c03
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Fri Apr 20 23:29:56 2018

Settings > Display: Use a single div

Using a separate name div breaks dragging when the name is dragged
and isn't necessary.

TBR=stevenjb@chromium.org

(cherry picked from commit 8cbea5ecd76e5ee419a9ca10c3e1566f19db62bb)

Bug:  833036 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ia9bcc8556018e9dd6d76e0dac6221ecbf85997c7
Reviewed-on: https://chromium-review.googlesource.com/1014624
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Weidong Guo <weidongg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#551426}
Reviewed-on: https://chromium-review.googlesource.com/1023055
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#184}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/9b9d2ce47a0f6d9849d8dde3393405d0e17a4c03/chrome/browser/resources/settings/device_page/display_layout.html
[modify] https://crrev.com/9b9d2ce47a0f6d9849d8dde3393405d0e17a4c03/chrome/browser/resources/settings/device_page/display_layout.js

Issue 842169 has been merged into this issue.