New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Task



Sign in to add a comment
link

Issue 882508: Clean up clank's compositor/layouts code

Reported by mdjones@chromium.org, Sep 10 Project Member

Issue description

This is a general tracking bug for cleaning up the compositor directory. There is a lot of unused and unneeded code that should be removed as well as layers of abstraction that can be made simpler.
 

Comment 1 by mdjones@chromium.org, Sep 10

Cc: yus...@chromium.org
+yusufo since this will affect the tab switcher.

Comment 2 by bugdroid1@chromium.org, Sep 11

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

commit c120ca1d1c52d1a55618c44b6b978c4377765f57
Author: Matthew Jones <mdjones@chromium.org>
Date: Tue Sep 11 21:48:29 2018

Remove unused independent animation from LayoutTab

This patch reomves some obsolete code that was used to transition
theme colors on the tab border in the tab switcher. This feature
has since been removed.

Bug: 882508
Change-Id: Ie63fabb08631f0377721f7057cbe65a4cd3af9a2
Reviewed-on: https://chromium-review.googlesource.com/1217405
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590488}
[modify] https://crrev.com/c120ca1d1c52d1a55618c44b6b978c4377765f57/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/Layout.java
[modify] https://crrev.com/c120ca1d1c52d1a55618c44b6b978c4377765f57/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/components/LayoutTab.java

Comment 3 by bugdroid1@chromium.org, Sep 12

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

commit b626cd1fa6e412ab1c9beb2d1eecfa3eb5ee3210
Author: Matthew Jones <mdjones@chromium.org>
Date: Wed Sep 12 17:09:08 2018

Convert animations in StaticLayout to CompositorAnimator

This patch migrates the remaining animations in StaticLayout to use
CompositorAnimator.

Bug: 882508, 750381
Change-Id: I240a45cfdbf634cd122a34825648e459f9de6b61
Reviewed-on: https://chromium-review.googlesource.com/1217406
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590732}
[modify] https://crrev.com/b626cd1fa6e412ab1c9beb2d1eecfa3eb5ee3210/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/StaticLayout.java
[modify] https://crrev.com/b626cd1fa6e412ab1c9beb2d1eecfa3eb5ee3210/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/components/LayoutTab.java

Comment 4 by bugdroid1@chromium.org, Sep 12

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5725cb4927d2266527e2750a3bdcaba15f77600a

commit 5725cb4927d2266527e2750a3bdcaba15f77600a
Author: Matthew Jones <mdjones@chromium.org>
Date: Wed Sep 12 18:43:52 2018

Move animation handling to Stack

This patch moves the remaining ChromeAnimation logic to the
StackLayout (the last place it is used). This allows us to start
simplifying the oversized Layout API.

Bug: 882508, 750381
Change-Id: I8b59cff1f8eb10777a524c078fadf008dec46542
Reviewed-on: https://chromium-review.googlesource.com/1217597
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590763}
[modify] https://crrev.com/5725cb4927d2266527e2750a3bdcaba15f77600a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/Layout.java
[modify] https://crrev.com/5725cb4927d2266527e2750a3bdcaba15f77600a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayoutBase.java

Comment 5 by bugdroid1@chromium.org, Sep 12

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

commit fffc2a9d17cb1af7edb6cc640b3171e25955f356
Author: Matthew Jones <mdjones@chromium.org>
Date: Wed Sep 12 22:11:49 2018

Remove a large amount of plumbing in UI compositor

This patch removes a large amount of extra plumbing. For example, the
layout manager previously added observers to the tabs and tab models
to proxy the events to the layout, which would then proxy them to
various scene overlays. Each class already has access to the tab model
selector, so this patch opts to have each class implement its own
observers for the methods it needs rather than doing all the plumbing.

The result of this is an API that matches many of the existing
observers instead of inventing new ones and slightly renaming them. In
general this is less confusing to reason about.

This is the first of a couple of patches like this.

Bug: 882508
Change-Id: Ic67644be9c3fa5736cef584aa4243c90019ee30b
Reviewed-on: https://chromium-review.googlesource.com/1219928
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590835}
[modify] https://crrev.com/fffc2a9d17cb1af7edb6cc640b3171e25955f356/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanel.java
[modify] https://crrev.com/fffc2a9d17cb1af7edb6cc640b3171e25955f356/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/Layout.java
[modify] https://crrev.com/fffc2a9d17cb1af7edb6cc640b3171e25955f356/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManager.java
[modify] https://crrev.com/fffc2a9d17cb1af7edb6cc640b3171e25955f356/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/StaticLayout.java
[modify] https://crrev.com/fffc2a9d17cb1af7edb6cc640b3171e25955f356/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayoutBase.java
[modify] https://crrev.com/fffc2a9d17cb1af7edb6cc640b3171e25955f356/chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/SceneOverlay.java
[modify] https://crrev.com/fffc2a9d17cb1af7edb6cc640b3171e25955f356/chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperManager.java
[modify] https://crrev.com/fffc2a9d17cb1af7edb6cc640b3171e25955f356/chrome/android/java/src/org/chromium/chrome/browser/compositor/scene_layer/ScrollingBottomViewSceneLayer.java
[modify] https://crrev.com/fffc2a9d17cb1af7edb6cc640b3171e25955f356/chrome/android/java/src/org/chromium/chrome/browser/compositor/scene_layer/ToolbarSceneLayer.java

Comment 6 by bugdroid1@chromium.org, Sep 14

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

commit d2aa4f06cf7ca6b34c4f0a138ff3e3863227401a
Author: Matthew Jones <mdjones@chromium.org>
Date: Fri Sep 14 16:23:07 2018

Remove largely unused event processing from layout system

Most layouts use an event filter to handle input. There exists a
second input handling system in layout that is now only used by the
toolbar swipe layout. This patch removes the unused plumbing and
isolates the code that toolbar swipe uses. Conversion to the newer
event filter system will happen in a later patch.

Bug: 882508
Change-Id: Ib8d80c34e75a8e587f8db65bbf996541970b7057
Reviewed-on: https://chromium-review.googlesource.com/1225881
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591365}
[delete] https://crrev.com/e1d4937ced2baf64469fc3262cc9d21f3d99aa25/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/EdgeSwipeHandlerLayoutDelegate.java
[modify] https://crrev.com/d2aa4f06cf7ca6b34c4f0a138ff3e3863227401a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/Layout.java
[modify] https://crrev.com/d2aa4f06cf7ca6b34c4f0a138ff3e3863227401a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChrome.java
[modify] https://crrev.com/d2aa4f06cf7ca6b34c4f0a138ff3e3863227401a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/ToolbarSwipeLayout.java
[modify] https://crrev.com/d2aa4f06cf7ca6b34c4f0a138ff3e3863227401a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayoutBase.java
[modify] https://crrev.com/d2aa4f06cf7ca6b34c4f0a138ff3e3863227401a/chrome/android/java_sources.gni

Comment 7 by bugdroid1@chromium.org, Sep 18

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

commit 9abcf9a285bf0620f5edeac3cf8590a846588e67
Author: Matthew Jones <mdjones@chromium.org>
Date: Tue Sep 18 15:34:43 2018

Fix tab switcher bottom toolbar animations

This patch only propagates swipe end and fling events to the toolbar
swipe layout if it is showing. Previously all touch events to interact
with the modified swipe handler would be processed regargless of which
layout was showing. The result was incorrect events being triggered
and, in this case, showing incorrect buttons in the bottom toolbar.

Bug: 884615, 882508
Change-Id: Id791624d1505a18c6fd16bf6a64544a049503d16
Reviewed-on: https://chromium-review.googlesource.com/1228936
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592048}
[modify] https://crrev.com/9abcf9a285bf0620f5edeac3cf8590a846588e67/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChrome.java

Comment 8 by bugdroid1@chromium.org, Oct 5

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/57eb9ffa5a6735904fb6f7aac64befa5131a59b4

commit 57eb9ffa5a6735904fb6f7aac64befa5131a59b4
Author: Matthew Jones <mdjones@chromium.org>
Date: Fri Oct 05 23:42:09 2018

Remove some dead code from Layout

Layout#usesToolbarLayer is never used and can therefore be removed.

Bug: 882508
Change-Id: Id255a67c696e47080e00f58e74500643f3ed04c8
Reviewed-on: https://chromium-review.googlesource.com/c/1265120
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597369}
[modify] https://crrev.com/57eb9ffa5a6735904fb6f7aac64befa5131a59b4/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/Layout.java

Comment 9 by bugdroid1@chromium.org, Jan 12

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/13033d79aa73172078a54302b02465718bc86836

commit 13033d79aa73172078a54302b02465718bc86836
Author: Matthew Jones <mdjones@chromium.org>
Date: Sat Jan 12 02:12:12 2019

Use px values for browser controls height and offset in java

This patch converts most of the float values that were previously
storing integers into proper integers and updates the related logic.
The idea here is to remove all of the unnecessary float comparison
and rounding that previously plagued the code; especially since all
of the values are tracked in pixels anyway (rather than DP).

Bug: 882508
Change-Id: I664fb52e3636aa99ac4937c9e3c1b29a4866eee6
Reviewed-on: https://chromium-review.googlesource.com/c/1404391
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622266}
[modify] https://crrev.com/13033d79aa73172078a54302b02465718bc86836/chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java
[modify] https://crrev.com/13033d79aa73172078a54302b02465718bc86836/chrome/android/java/src/org/chromium/chrome/browser/fullscreen/FullscreenManager.java
[modify] https://crrev.com/13033d79aa73172078a54302b02465718bc86836/chrome/android/java/src/org/chromium/chrome/browser/tab/TabBrowserControlsOffsetHelper.java
[modify] https://crrev.com/13033d79aa73172078a54302b02465718bc86836/chrome/android/java/src/org/chromium/chrome/browser/tab/TabViewAndroidDelegate.java
[modify] https://crrev.com/13033d79aa73172078a54302b02465718bc86836/chrome/android/java/src/org/chromium/chrome/browser/toolbar/bottom/BrowsingModeBottomToolbarViewBinder.java
[modify] https://crrev.com/13033d79aa73172078a54302b02465718bc86836/chrome/android/javatests/src/org/chromium/chrome/browser/fullscreen/FullscreenManagerTest.java
[modify] https://crrev.com/13033d79aa73172078a54302b02465718bc86836/chrome/android/javatests/src/org/chromium/chrome/browser/fullscreen/FullscreenManagerTestUtils.java
[modify] https://crrev.com/13033d79aa73172078a54302b02465718bc86836/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java

Comment 10 by bugdroid1@chromium.org, Jan 17

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f42281b9363d934e9b38e3ca53d91c3a950b38c

commit 7f42281b9363d934e9b38e3ca53d91c3a950b38c
Author: Matthew Jones <mdjones@chromium.org>
Date: Thu Jan 17 18:51:00 2019

Use px for units in browser controls offsets in native code

This patch removes two back and forth unit conversions in Tab's java
code and then in the RenderWidgetHostImpl, making px the unit used
to describe browser controls offsets and height everywhere.

Bug: 882508
Change-Id: I2fba83f659021f7c448d5012a0b3c7b16a20671c
Reviewed-on: https://chromium-review.googlesource.com/c/1407920
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623772}
[modify] https://crrev.com/7f42281b9363d934e9b38e3ca53d91c3a950b38c/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/7f42281b9363d934e9b38e3ca53d91c3a950b38c/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/7f42281b9363d934e9b38e3ca53d91c3a950b38c/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/7f42281b9363d934e9b38e3ca53d91c3a950b38c/content/public/test/android/dom_utils.cc

Comment 11 by bugdroid1@chromium.org, Jan 22

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/876b887aab85a440359f38bf6d08630bcde633c8

commit 876b887aab85a440359f38bf6d08630bcde633c8
Author: Matthew Jones <mdjones@chromium.org>
Date: Tue Jan 22 17:53:31 2019

Clean up fullscreen manager interfaces post-unit conversion

This patch changes the units of the FullscreenListener interface to
be consistent with those that use it. This patch also contains some
cosmetic changes and removal of unnecessary casts.

Bug: 882508
Change-Id: Ia4c808a00f97a74dc6dfc497d0b4c4e833655389
Reviewed-on: https://chromium-review.googlesource.com/c/1409488
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624826}
[modify] https://crrev.com/876b887aab85a440359f38bf6d08630bcde633c8/chrome/android/java/src/org/chromium/chrome/browser/autofill_assistant/TouchEventFilterView.java
[modify] https://crrev.com/876b887aab85a440359f38bf6d08630bcde633c8/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java
[modify] https://crrev.com/876b887aab85a440359f38bf6d08630bcde633c8/chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsMediator.java
[modify] https://crrev.com/876b887aab85a440359f38bf6d08630bcde633c8/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabBottomBarDelegate.java
[modify] https://crrev.com/876b887aab85a440359f38bf6d08630bcde633c8/chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java
[modify] https://crrev.com/876b887aab85a440359f38bf6d08630bcde633c8/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/indicator/TopSnackbarManager.java
[modify] https://crrev.com/876b887aab85a440359f38bf6d08630bcde633c8/chrome/android/java/src/org/chromium/chrome/browser/snackbar/BottomContainer.java
[modify] https://crrev.com/876b887aab85a440359f38bf6d08630bcde633c8/chrome/android/java/src/org/chromium/chrome/browser/toolbar/bottom/BrowsingModeBottomToolbarMediator.java
[modify] https://crrev.com/876b887aab85a440359f38bf6d08630bcde633c8/chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheet.java
[modify] https://crrev.com/876b887aab85a440359f38bf6d08630bcde633c8/chrome/android/javatests/src/org/chromium/chrome/browser/fullscreen/FullscreenManagerTest.java
[modify] https://crrev.com/876b887aab85a440359f38bf6d08630bcde633c8/chrome/android/javatests/src/org/chromium/chrome/browser/fullscreen/FullscreenManagerTestUtils.java

Sign in to add a comment