New issue
Advanced search Search tips

Issue 803326 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 800502



Sign in to add a comment

[MD Extensions] Migrate PolicyTest.DeveloperToolsDisabledExtensionsDevMode to new UI

Project Member Reported by dpa...@chromium.org, Jan 18 2018

Issue description

I have been attempting to port this test over to the new UI, unsuccessfully for a few hours now. It seems that the toolbar drawer animation is causing a race condition that interferes with the test. 

I'll follow up with more details. As a starting point, discovered that the logic at [1] ends up calling setProfileInDevMode() twice, since there are always two event occurrences for a single user click, one for 'top' and one for 'height'.

[1] https://cs.chromium.org/chromium/src/chrome/browser/resources/md_extensions/toolbar.js?l=69-71
 

Comment 1 by dpa...@chromium.org, Jan 18 2018

Blocking: 800502
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 19 2018

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

commit 9074e59e8656131782e604573214b722ae5024b4
Author: dpapad <dpapad@chromium.org>
Date: Fri Jan 19 04:02:46 2018

MD Extensions: Modify toolbar inDevMode toggle logic.

Before this CL
1) for each user interaction there were 2 JS->C++ messages,
   instead of just 1.
2) the C++ call was made only after the drawer animation finished,
   which created a race condition between the inDevMode boolean and
   the toggle's state. Now it happens as soon as the user clicks.
3) The height of the cards was snapping after the toolbar animation
   finished. No longer the case. The height of the cards animates at
   the same time with the toolbar.

This simplifies the logic a bit, and unblocks porting the
PolicyTest.DeveloperToolsDisabledExtensionsDevMode to the new UI.

Bug:  803326 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I4f2995bc322f619fbc8fad812dd03344e895391c
Reviewed-on: https://chromium-review.googlesource.com/874775
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530420}
[modify] https://crrev.com/9074e59e8656131782e604573214b722ae5024b4/chrome/browser/resources/md_extensions/compiled_resources2.gyp
[modify] https://crrev.com/9074e59e8656131782e604573214b722ae5024b4/chrome/browser/resources/md_extensions/item.html
[modify] https://crrev.com/9074e59e8656131782e604573214b722ae5024b4/chrome/browser/resources/md_extensions/toolbar.html
[modify] https://crrev.com/9074e59e8656131782e604573214b722ae5024b4/chrome/browser/resources/md_extensions/toolbar.js

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 22 2018

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

commit d74544f69ecbcfa29f2587e1ff63daddd0e6e490
Author: dpapad <dpapad@chromium.org>
Date: Mon Jan 22 19:26:03 2018

MD Extensions: Port PolicyTest.DeveloperToolsDisabledExtensionsDevMode to new UI.

Bug:  803326 
Change-Id: I65907b9151c940421273f73171dbd1322ad9e571
Reviewed-on: https://chromium-review.googlesource.com/876730
Reviewed-by: Drew Wilson <atwilson@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530945}
[modify] https://crrev.com/d74544f69ecbcfa29f2587e1ff63daddd0e6e490/chrome/browser/policy/policy_browsertest.cc

Comment 4 by dpa...@chromium.org, Jan 22 2018

Labels: Merge-Request-65
Requesting merge for https://chromium-review.googlesource.com/874775, which adds a new animation when entering/exiting dev mode.

Comment 5 by dpa...@chromium.org, Jan 22 2018

Status: Fixed (was: Assigned)
Project Member

Comment 6 by sheriffbot@chromium.org, Jan 23 2018

Labels: -Merge-Request-65 Hotlist-Merge-Approved Merge-Approved-65
Your change meets the bar and is auto-approved for M65. Please go ahead and merge the CL to branch 3325 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

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

Comment 7 by gov...@chromium.org, Jan 23 2018

Pls merge your change to M65 branch 3325 before 2:00 PM PT today, Tuesday (01/23/18) so we can pick it up for dev release tomorrow. Thank you.
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 23 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0cfa60ee604216433d1b430acf9d5c417f6ce4ee

commit 0cfa60ee604216433d1b430acf9d5c417f6ce4ee
Author: dpapad <dpapad@chromium.org>
Date: Tue Jan 23 21:39:16 2018

[M65 merge] MD Extensions: Modify toolbar inDevMode toggle logic.

Before this CL
1) for each user interaction there were 2 JS->C++ messages,
   instead of just 1.
2) the C++ call was made only after the drawer animation finished,
   which created a race condition between the inDevMode boolean and
   the toggle's state. Now it happens as soon as the user clicks.
3) The height of the cards was snapping after the toolbar animation
   finished. No longer the case. The height of the cards animates at
   the same time with the toolbar.

This simplifies the logic a bit, and unblocks porting the
PolicyTest.DeveloperToolsDisabledExtensionsDevMode to the new UI.

TBR=dpapad@chromium.org

(cherry picked from commit 9074e59e8656131782e604573214b722ae5024b4)

Bug:  803326 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I4f2995bc322f619fbc8fad812dd03344e895391c
Reviewed-on: https://chromium-review.googlesource.com/874775
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#530420}
Reviewed-on: https://chromium-review.googlesource.com/881868
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#37}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/0cfa60ee604216433d1b430acf9d5c417f6ce4ee/chrome/browser/resources/md_extensions/compiled_resources2.gyp
[modify] https://crrev.com/0cfa60ee604216433d1b430acf9d5c417f6ce4ee/chrome/browser/resources/md_extensions/item.html
[modify] https://crrev.com/0cfa60ee604216433d1b430acf9d5c417f6ce4ee/chrome/browser/resources/md_extensions/toolbar.html
[modify] https://crrev.com/0cfa60ee604216433d1b430acf9d5c417f6ce4ee/chrome/browser/resources/md_extensions/toolbar.js

Sign in to add a comment