New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 685831 link

Starred by 1 user
Status: Verified
Owner:
█ warx@chromium.org
Last visit > 30 days ago
Closed: Feb 2017
Cc:
yiyix@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Remove the non-MD immersive mode code paths

Project Member Reported by tdander...@chromium.org, Jan 26 2017 
The material design version of immersive mode was enabled by default in M-55 (see issue 641951), so we can now remove the non-MD code paths. Relevant code includes:

ImmersiveModeControllerAsh
TabStrip
BrowserNonClientFrameViewAsh
ShelfLayoutManager
Removal of MaterialDesignController::IsImmersiveModeMaterial()

Comment 1 by tdander...@chromium.org, Jan 28 2017 

Also, the TODO in BrowserNonClientFrameViewAsh::ResetWindowControls() should be addressed as part of this bug.

Comment 2 by tdander...@chromium.org, Feb 7 2017

Labels: Hotlist-CodeHealth

Comment 3 by tdander...@chromium.org, Feb 14 2017

Cc: yiyix@chromium.org
Owner: warx@chromium.org
Reassigning to warx as per discussion in https://bugs.chromium.org/p/chromium/issues/detail?id=614453#c9

Comment 4 by warx@chromium.org, Feb 14 2017

Labels: M-57
Thanks tdanderson@. I created a CL https://codereview.chromium.org/2690443002/ that we wanted to target for M-57, which has

ImmersiveModeControllerAsh
TabStrip
BrowserNonClientFrameViewAsh

non-MD immersive mode code paths removed checked.

Comment 5 by tdander...@chromium.org, Feb 14 2017

Labels: -M-58
Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 16 2017 

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

commit f05f264d80c72ec2681cbd4b3854ab6ceceaee67
Author: warx <warx@chromium.org>
Date: Thu Feb 16 18:43:20 2017

cros-md: Remove the non-MD immersive mode code paths

Changes:
Remove the non-MD immersive mode code paths

BUG= 690295 
BUG= 685831 
TEST=emulator test, automation tests, also add test coverage

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

[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/ash/common/material_design/material_design_controller.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/ash/common/material_design/material_design_controller.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/ash/common/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_view_layout.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_view_layout_unittest.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/immersive_mode_controller.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/immersive_mode_controller_ash.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/immersive_mode_controller_ash_unittest.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/immersive_mode_controller_stub.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/immersive_mode_controller_stub.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab_controller.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab_strip.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab_strip.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab_strip_unittest.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab_unittest.cc

Comment 7 by warx@chromium.org, Feb 16 2017

Status: Fixed (was: Started)
ImmersiveModeControllerAsh
TabStrip
BrowserNonClientFrameViewAsh
ShelfLayoutManager
Removal of MaterialDesignController::IsImmersiveModeMaterial()

are all checked in the above CL, marked as fixed since merging to M57 is tracked in  issue 690295 .
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 17 2017

Labels: merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a775b78d6cc7185af83e1095d6919505b3b7cba

commit 9a775b78d6cc7185af83e1095d6919505b3b7cba
Author: Qiang Xu <warx@chromium.org>
Date: Fri Feb 17 00:31:17 2017

[Merge to M57] cros-md: Remove the non-MD immersive mode code paths

Changes:
Remove the non-MD immersive mode code paths

TBR=tdanderson@chromium.org, sky@chromium.org
BUG= 690295 
BUG= 685831 
TEST=emulator test, automation tests, also add test coverage

Review-Url: https://codereview.chromium.org/2690443002
Cr-Commit-Position: refs/heads/master@{#451022}
(cherry picked from commit f05f264d80c72ec2681cbd4b3854ab6ceceaee67)

Review-Url: https://codereview.chromium.org/2702663002 .
Cr-Commit-Position: refs/branch-heads/2987@{#567}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/ash/common/material_design/material_design_controller.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/ash/common/material_design/material_design_controller.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/ash/common/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_view_layout.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_view_layout_unittest.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/immersive_mode_controller.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/immersive_mode_controller_ash.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/immersive_mode_controller_ash_unittest.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/immersive_mode_controller_stub.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/immersive_mode_controller_stub.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab_controller.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab_strip.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab_strip.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab_strip_unittest.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab_unittest.cc

Comment 9 by rookrishna@chromium.org, Mar 1 2017

Status: Verified (was: Fixed)
►

Sign in to add a comment