Skip to content

GitLab

T
tangled
Switch branch/tag
  1. 25 May, 2018 33 commits
  3. 24 May, 2018 7 commits
    • danakj's avatar
      Move LayerTreeHostImpl-dependent code in viz/client/ to cc/. · 54af81af
      danakj authored 
      Reland of https://chromium-review.googlesource.com/c/chromium/src/+/1066987

Currently there is a HitTestDataProviderSurfaceLayer class in
components/viz/client, which receives a cc::LayerTreeHostImpl* from cc/
and then calls back into cc::LayerTreeHostImpl* to build the
HitTestRegionList from the active tree. This is problematic for these
reasons:

1. cc is-a viz client, so it could depend on viz/client/ code. But the
viz/client/ should not depend back on cc/. The use of the
cc::LayerTreeHostImpl pointer breaks this.

2. The LayerTreeFrameSink implementation can call back to
LayerTreeHostImpl, but it should not do so with the actual LTHI pointer
type. Instead it should use the LayerTreeFrameSinkClient interface,
which LayerTreeHostImpl implements.

3. The order of exection is that cc::LayerTreeHostImpl calls
ClientLayerTreeFrameSink::SubmitCompositorFrame() (through the
LayerTreeFrameSink interface), which then calls back to the
cc::LayerTreeHostImpl class to generate the HitTestRegionList,
creating the A-B-A anti-pattern. We can resolve this by passing
the HitTestRegionList directly to SubmitCompositorFrame(), but to do
so we need ClientLayerTreeFrameSink to not get it from the virtual
HitTestDataProvider interface in this case.

In order to satisfy these things we:

A. Move the implementation of HitTestDataProviderSurfaceLayer into
LayerTreeHostImpl, and move the unit test there.

B. Add a BuildHitTestData() method to LayerTreeFrameSinkClient, which
is implemented in LayerTreeHostImpl as per (A).

C. Move the branching out of the HitTestDataProvider virtuality.
If the HitTestDataProvider in ClientLayerTreeFrameSink is null, then
it uses the LayerTreeFrameSinkClient to ask the LayerTreeHostImpl to
build the HitTestRegionList. This does not break behaviour as the
HitTestDataProvider is created as a HitTestDataProviderDrawQuad
preferably, falling back to HitTestDataProviderSurfaceLayer.
Similarly the HitTestDataProviderDrawQuad would be preferred if the
pointer is not null, falling back to using LayerTreeFrameSinkClient.
(We also leave a TODO that we can just submit the HitTestRegionList
directly to SubmitCompositorFrame.)

This patch would be pretty small.. except..

Because cc/ is building the HitTestRegionList, it can not depend
on services/viz/ which depends back on cc/. So we implement the
viz::mojom::HitTestRegionList and viz::mojom::HitTestRegion as
standard c++ types viz::HitTestRegionList and viz::HitTestRegion
in components/viz/common/hit_test/, which cc/ can make use of. And
we add struct traits to pass these types to mojo and receive them
from mojo.

Then all plumbing code must be changed from the mojom-defined type
to the standard-c++-defined type.

The mojom type was viz::mojom::HitTestRegionListPtr (aka
mojo::StructPtr<viz::mojom::HitTestRegionList>), which is an
optional field in the CompositorFrame, so we must preserve its
ability to be null. We may use a struct traits directly to
viz::HitTestRegionList, and base::Optional<viz::HitTestRegionList>
throughout the plumbing, or we can use a struct traits to a
std::unique_ptr<viz::HitTestRegionList> (which is itself considered
nullable by mojo) and plumb that through directly through
everything. I don't actually know which is preferable here, but
settled on making viz::HitTestRegionList move-only, and using the
former option. This makes most of this patch just type renames.

Lastly, we need LayerTreeHostImpl to not return a HitTestRegionList
in cases where no HitTestDataProvider would have been given to the
ClientLayerTreeFrameSink previously. This is in the case where
features::IsVizHitTestingSurfaceLayerEnabled() returns false. So we
add a field to LayerTreeSettings, which we set to the value of that
feature flag, and have LayerTreeHostImpl return an empty optional
value when the setting is false.

TBR=gklassen@chromium.org, sadrul@chromium.org, reveman@chromium.org, avi@chromium.org, dcheng@chromium.org

Bug: 722935
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Iba36b0cb12e2e45b80211956fb932c26693d1cd3
Reviewed-on: https://chromium-review.googlesource.com/1072128Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatardanakj <danakj@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561697}
      54af81af
    • Sarah Hu's avatar
      Interagion with the processor stub and time limit pref · 376503e2
      Sarah Hu authored 
      Change-Id: I819742fc61d41ffdeb00b5ccd888a1fe56feb3d2
Bug: 823536
Reviewed-on: https://chromium-review.googlesource.com/1068659Reviewed-by: default avatarRahul Chaturvedi <rkc@chromium.org>
Reviewed-by: default avatarJacob Dufault <jdufault@chromium.org>
Commit-Queue: Xiaoyin Hu <xiaoyinh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561696}
      376503e2
    • Noel Gordon's avatar
      #define DISABLE_SLOW_FILESAPP_TESTS is no longer used: remove · dae6df8a
      Noel Gordon authored 
      Woohoo, I suppose.

Bug: 836254
Change-Id: Ic5aa72305b0884cfee57f77a66339bf0d83f3cd9
Reviewed-on: https://chromium-review.googlesource.com/1071029Reviewed-by: default avatarStuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561695}
      dae6df8a
    • Gabriel Charette's avatar
      Revert "Add a LocalSiteCharacteristicsDataStore factory" · 48b45859
      Gabriel Charette authored 
      This reverts commit 6a1485d2.

Reason for revert: breaks Linux CFI, crbug.com/846492

Original change's description:
> Add a LocalSiteCharacteristicsDataStore factory
> 
> This follows the guidelines from this page: https://www.chromium.org/developers/design-documents/profile-architecture
> 
> Bug: 773382
> Change-Id: I8517975f9881e3fa2d52fb26b16008d780255eef
> Reviewed-on: https://chromium-review.googlesource.com/1047725
> Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
> Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#561176}

TBR=bauerb@chromium.org,fdoray@chromium.org,erg@chromium.org,sebmarchand@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 773382
Change-Id: I9216725dd36891661b75a3d03a93d5298ea2dc30
Reviewed-on: https://chromium-review.googlesource.com/1072787
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561694}
      48b45859
    • depot-tools-chromium-autoroll's avatar
      Roll src/third_party/depot_tools/ b1a5e452d..af2ffd933 (1 commit) · f51a8718
      depot-tools-chromium-autoroll authored 
      https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/b1a5e452d31e..af2ffd933d0e

$ git log b1a5e452d..af2ffd933 --date=short --no-merges --format='%ad %ae %s'
2018-05-24 agable Remove cq_client from depot_tools

Created with:
  roll-dep src/third_party/depot_tools
BUG=chromium:770408


The AutoRoll server is located here: https://depot-tools-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


TBR=agable@chromium.org

Change-Id: Ibd05c61891c11981e0aa3546b2aed95be31d6916
Reviewed-on: https://chromium-review.googlesource.com/1072687Reviewed-by: default avatardepot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#561693}
      f51a8718
    • David Jacobo's avatar
      Set a valid Referrer for ArcExternalProtocolDialog · 4ebf0ac4
      David Jacobo authored 
      Constructing a basic default Referrer for ArcExternalProtocolDialog
based on the last committed url.

Bug: None
Test: Build.
Change-Id: I6aeb677d57632198b69c7cc229f92a2571526a0c
Reviewed-on: https://chromium-review.googlesource.com/1072513Reviewed-by: default avatarYusuke Sato <yusukes@chromium.org>
Commit-Queue: David Jacobo <djacobo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561692}
      4ebf0ac4
    • Hector Carmona's avatar
      WebUI: Improve A11y by autofocusing the back button. · 8fd1d095
      Hector Carmona authored 
      This affects both chrome://settings and chrome://extensions.

Bug: 840633
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I5e69e542a9c08fd35c2f6b02a8277d7ee87998fe
Reviewed-on: https://chromium-review.googlesource.com/1068114
Commit-Queue: Hector Carmona <hcarmona@chromium.org>
Reviewed-by: default avatarDemetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561691}
      8fd1d095