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

Issue 859977 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Unify DelegatedFrameHost and DelegatedFrameHostAndroid

Project Member Reported by fsam...@chromium.org, Jul 3

Issue description

These classes are very similar but not quite the same. We should unify them. 
 
Hi, I am willing to help on this, and my WIP CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1168740
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 14

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

commit 5edb31a0cdb1f17b7eb52cb0fe811077226ac9b9
Author: Xu Xing <xing.xu@intel.com>
Date: Tue Aug 14 14:44:18 2018

viz: Unify DelegatedFrameHost and DelegatedFrameHostAndroid

Main changes:
1), move flag enable viz inside DelegatedFrameHost.
2), cache host_frame_sink_manager_, so this matches DelegatedFrameHostAndroid
and saves several lines of code.
3), if enable_viz_ is true, support_ is nullptr. Otherwise, support_ is valid.
So (enable_viz_ || support_ ) is always true.

Bug: 859977
Change-Id: If13d08dc7dc1ba04de507d181bf2a940d8010466
Reviewed-on: https://chromium-review.googlesource.com/1168740
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Xing Xu <xing.xu@intel.com>
Cr-Commit-Position: refs/heads/master@{#582911}
[modify] https://crrev.com/5edb31a0cdb1f17b7eb52cb0fe811077226ac9b9/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/5edb31a0cdb1f17b7eb52cb0fe811077226ac9b9/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/5edb31a0cdb1f17b7eb52cb0fe811077226ac9b9/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/5edb31a0cdb1f17b7eb52cb0fe811077226ac9b9/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/5edb31a0cdb1f17b7eb52cb0fe811077226ac9b9/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

Owner: xing...@intel.com
Status: Started (was: Untriaged)
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 21

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

commit 9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6
Author: Xu Xing <xing.xu@intel.com>
Date: Tue Aug 21 00:11:20 2018

DelegatedFrameHost has no need to know RenderWidgetHostViewBase

Also refactor HasFallbackSurface to unify DelegatedFrameHost and
DelegatedFrameHostAndroid.

Bug: 859977
Change-Id: I5cea0013b4f52dbe4b6c8a6a9841bacc7b13e381
Reviewed-on: https://chromium-review.googlesource.com/1176970
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Xing Xu <xing.xu@intel.com>
Cr-Commit-Position: refs/heads/master@{#584584}
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/content/browser/renderer_host/render_widget_host_ns_view_client.h
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/9ef5a887c3f936d424e7d20f43bcc69f65c3c9f6/ui/android/delegated_frame_host_android.h

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 25

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

commit 6eeb35a312825278c902bcd0b3a4e9485f5621bd
Author: Xu Xing <xing.xu@intel.com>
Date: Sat Aug 25 00:24:14 2018

Implement DelegatedFrameHostClientAndroid

This behaves similar to DelegatedFrameHost. As discussed in
https://chromium-review.googlesource.com/c/chromium/src/+/1173663,
this implements composition instead of inheritance.

Bug: 859977
Change-Id: Icf5e0c83c4b3789a2173933a26067c6275f62765
Reviewed-on: https://chromium-review.googlesource.com/1186199
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Xing Xu <xing.xu@intel.com>
Cr-Commit-Position: refs/heads/master@{#586077}
[modify] https://crrev.com/6eeb35a312825278c902bcd0b3a4e9485f5621bd/content/browser/BUILD.gn
[add] https://crrev.com/6eeb35a312825278c902bcd0b3a4e9485f5621bd/content/browser/renderer_host/delegated_frame_host_client_android.cc
[add] https://crrev.com/6eeb35a312825278c902bcd0b3a4e9485f5621bd/content/browser/renderer_host/delegated_frame_host_client_android.h
[modify] https://crrev.com/6eeb35a312825278c902bcd0b3a4e9485f5621bd/content/browser/renderer_host/delegated_frame_host_client_aura.cc
[modify] https://crrev.com/6eeb35a312825278c902bcd0b3a4e9485f5621bd/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/6eeb35a312825278c902bcd0b3a4e9485f5621bd/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/6eeb35a312825278c902bcd0b3a4e9485f5621bd/ui/android/delegated_frame_host_android.h

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 30

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

commit 816c20c71f46a72daac1e7485fe513763d9b55a4
Author: Xu Xing <xing.xu@intel.com>
Date: Thu Aug 30 01:44:47 2018

Rename APIs in DelegatedFrameHost and DelegatedFrameHostAndroid

For DelegatedFrameHost, ResetCompositor is used to disconnect from
compositor:
SetCompositor=>AttachToCompositor
ResetCompositor=>DetachFromCompositor

For compositor, AddFrameSink adds frame sink id into child_frame_sinks_:
AddFrameSink=>AddChildFrameSink
RemoveFrameSink=>RemoveChildFrameSink

For DelegatedFrameHostAndroid:
CreateNewCompositorFrameSinkSupport=>CreateCompositorFrameSinkSupport

Also wrap Android version check into IsResizeLockEnabled.

Bug: 859977
Change-Id: Id878a04c49cab23e1d585cd1cca893d601fb70d8
Reviewed-on: https://chromium-review.googlesource.com/1186190
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587402}
[modify] https://crrev.com/816c20c71f46a72daac1e7485fe513763d9b55a4/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/816c20c71f46a72daac1e7485fe513763d9b55a4/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/816c20c71f46a72daac1e7485fe513763d9b55a4/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/816c20c71f46a72daac1e7485fe513763d9b55a4/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/816c20c71f46a72daac1e7485fe513763d9b55a4/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/816c20c71f46a72daac1e7485fe513763d9b55a4/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/816c20c71f46a72daac1e7485fe513763d9b55a4/ui/android/delegated_frame_host_android.h
[modify] https://crrev.com/816c20c71f46a72daac1e7485fe513763d9b55a4/ui/android/delegated_frame_host_android_unittest.cc
[modify] https://crrev.com/816c20c71f46a72daac1e7485fe513763d9b55a4/ui/aura/window.cc
[modify] https://crrev.com/816c20c71f46a72daac1e7485fe513763d9b55a4/ui/compositor/compositor.cc
[modify] https://crrev.com/816c20c71f46a72daac1e7485fe513763d9b55a4/ui/compositor/compositor.h

Sign in to add a comment