New issue
Advanced search Search tips

Issue 658410 link

Starred by 3 users

Issue metadata

Status: Archived
Owner:
Closed: Nov 2016
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

DesktopCastingWarningView needs updating

Project Member Reported by est...@chromium.org, Oct 21 2016

Issue description

seems to come up when you're screen sharing and switch users. The code uses a bad style (bespoke title, padding, etc.). It seems to be copy-pasted from TeleportWarningView/MultiprofilesIntroView* and needs to reuse more base dialog functionality.

*two dialogs I'm currently updating
 

Comment 1 by est...@chromium.org, Oct 21 2016

here are screenshots for updated TeleportWarningView/MultiprofilesIntroView
puBE3aSaAif.png
108 KB View Download
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 2 2016

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

commit bca985a76bd8a84c1580e09cc4c621b499e2530b
Author: kjellander <kjellander@chromium.org>
Date: Wed Nov 02 12:34:44 2016

Revert of Use a standard message box for DesktopCastingWarningView. (patchset #5 id:80001 of https://codereview.chromium.org/2441403003/ )

Reason for revert:
Speculative revert for flaky interactive_ui_tests on Mac10.10 Tests. See  crbug.com/661550 .

Original issue's description:
> Use a standard message box for DesktopCastingWarningView.
>
> This is even simpler than what I did for TeleportWarningView, which
> means I should probably go back and re-rewrite that one.
>
> BUG= 658410 
>
> Committed: https://crrev.com/5f1173f1e5a5678cc6c7cc9046939447398eaf19
> Cr-Commit-Position: refs/heads/master@{#428440}

TBR=derat@chromium.org,msw@chromium.org,estade@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 658410 ,  661550 

Review-Url: https://codereview.chromium.org/2469853003
Cr-Commit-Position: refs/heads/master@{#429256}

[modify] https://crrev.com/bca985a76bd8a84c1580e09cc4c621b499e2530b/chrome/browser/ui/ash/multi_user/user_switch_util.cc
[modify] https://crrev.com/bca985a76bd8a84c1580e09cc4c621b499e2530b/chrome/browser/ui/ash/multi_user/user_switch_util.h
[modify] https://crrev.com/bca985a76bd8a84c1580e09cc4c621b499e2530b/chrome/browser/ui/ash/multi_user/user_switch_util_unittest.cc
[modify] https://crrev.com/bca985a76bd8a84c1580e09cc4c621b499e2530b/chrome/browser/ui/cocoa/simple_message_box_mac.mm
[modify] https://crrev.com/bca985a76bd8a84c1580e09cc4c621b499e2530b/chrome/browser/ui/simple_message_box.h
[modify] https://crrev.com/bca985a76bd8a84c1580e09cc4c621b499e2530b/chrome/browser/ui/views/simple_message_box_views.cc
[modify] https://crrev.com/bca985a76bd8a84c1580e09cc4c621b499e2530b/components/constrained_window/constrained_window_views.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 2 2016

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

commit 2befbdd78b547659e47298b68462f763404962c9
Author: kjellander <kjellander@chromium.org>
Date: Wed Nov 02 14:09:16 2016

Reland of Use a standard message box for DesktopCastingWarningView. (patchset #1 id:1 of https://codereview.chromium.org/2469853003/ )

Reason for revert:
Relanding as a new build showed the same flake: https://build.chromium.org/p/chromium.mac/builders/Mac10.10%20Tests/builds/8584

Original issue's description:
> Revert of Use a standard message box for DesktopCastingWarningView. (patchset #5 id:80001 of https://codereview.chromium.org/2441403003/ )
>
> Reason for revert:
> Speculative revert for flaky interactive_ui_tests on Mac10.10 Tests. See  crbug.com/661550 .
>
> Original issue's description:
> > Use a standard message box for DesktopCastingWarningView.
> >
> > This is even simpler than what I did for TeleportWarningView, which
> > means I should probably go back and re-rewrite that one.
> >
> > BUG= 658410 
> >
> > Committed: https://crrev.com/5f1173f1e5a5678cc6c7cc9046939447398eaf19
> > Cr-Commit-Position: refs/heads/master@{#428440}
>
> TBR=derat@chromium.org,msw@chromium.org,estade@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG= 658410 ,  661550 
>
> Committed: https://crrev.com/bca985a76bd8a84c1580e09cc4c621b499e2530b
> Cr-Commit-Position: refs/heads/master@{#429256}

TBR=derat@chromium.org,msw@chromium.org,estade@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 658410 ,  661550 

Review-Url: https://codereview.chromium.org/2468873005
Cr-Commit-Position: refs/heads/master@{#429269}

[modify] https://crrev.com/2befbdd78b547659e47298b68462f763404962c9/chrome/browser/ui/ash/multi_user/user_switch_util.cc
[modify] https://crrev.com/2befbdd78b547659e47298b68462f763404962c9/chrome/browser/ui/ash/multi_user/user_switch_util.h
[modify] https://crrev.com/2befbdd78b547659e47298b68462f763404962c9/chrome/browser/ui/ash/multi_user/user_switch_util_unittest.cc
[modify] https://crrev.com/2befbdd78b547659e47298b68462f763404962c9/chrome/browser/ui/cocoa/simple_message_box_mac.mm
[modify] https://crrev.com/2befbdd78b547659e47298b68462f763404962c9/chrome/browser/ui/simple_message_box.h
[modify] https://crrev.com/2befbdd78b547659e47298b68462f763404962c9/chrome/browser/ui/views/simple_message_box_views.cc
[modify] https://crrev.com/2befbdd78b547659e47298b68462f763404962c9/components/constrained_window/constrained_window_views.cc

Status: Fixed (was: Assigned)
TeleportWarningView/MultiprofilesIntroView could use the simple message box stuff, but they have a checkbox AND ask a question, which isn't currently supported. I don't plan to chase this rabbit hole any more, so I'm marking this fixed.

Comment 7 by dchan@google.com, Jan 21 2017

Labels: VerifyIn-57

Comment 8 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 9 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 10 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 12 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment