Skip to content

GitLab

T
tangled
Switch branch/tag
  1. 13 Apr, 2020 17 commits
  3. 12 Apr, 2020 18 commits
  5. 11 Apr, 2020 5 commits
    • chromium-internal-autoroll's avatar
      Roll src-internal 607540bcad8c..16297237f8cc (2 commits) · 61db434b
      chromium-internal-autoroll authored 
      https://chrome-internal.googlesource.com/chrome/src-internal.git/+log/607540bcad8c..16297237f8cc


Created with:
  gclient setdep -r src-internal@16297237f8cc

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://skia-autoroll.corp.goog/r/src-internal-chromium-autoroll
Please CC nhiroki@google.com,reillyg@google.com on the revert to ensure that a human
is aware of the problem.

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

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

Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Bug: None
Tbr: nhiroki@google.com,reillyg@google.com
Change-Id: I2e7fa54ff227d5104892511d915ee86ad48490c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2146514Reviewed-by: default avatarchromium-internal-autoroll <chromium-internal-autoroll@skia-corp.google.com.iam.gserviceaccount.com>
Commit-Queue: chromium-internal-autoroll <chromium-internal-autoroll@skia-corp.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#758415}
      61db434b
    • David Bokan's avatar
      Reland "Overlay scrollbars work correctly without overflow" · c4f8ded9
      David Bokan authored 
      This reverts commit 77eb87e6.

Reason for revert: Relanding now that we have data from UMA

Original change's description:
> Revert "Overlay scrollbars work correctly without overflow"
>
> This reverts commit 2c6eb18b.
>
> Reason for revert: Reverting temporarily to confirm effect on metrics
>
> Original change's description:
> > Overlay scrollbars work correctly without overflow
> >
> > Overlay scrollbars - the type that fade in/out, not overflow: overlay -
> > should only ever be visible when the content is scrolling. Therefore, if
> > the content cannot scroll, we should never see an overlay scrollbar.
> > This is different from classic (i.e. non-overlay) scrollbars. An
> > overflow:scroll scroller requires showing a (disabled) scrollbar even
> > when there is no overflow. Today, when an overlay scrollbar is created
> > we check after-the-fact in UpdateAfterLayout whether there is overflow
> > and, if there isn't, we remove overlay scrollbars. This is how the above
> > mentioned behavior for |overflow:scroll| overlays is implemented.
> >
> > However, this doesn't account for different kinds of updates.
> > UpdateAfterStyleChange must also recompute scrollbar states and didn't
> > have this no-overflow removal logic. If content caused a style update
> > that didn't require a layout, an overlay scrollbar would be attached in
> > this case. In the attached bug, the scrollbar is visible because the
> > fade out (on the main thread) is controlled by a timer that causes
> > scrollbars to be disabled (which causes them not to paint). In addition
> > to incorrectly creating the scrollbar, UpdateAfterStyleChange does not
> > compute the scrollbar enabled state so it defaults to true. Without
> > being able to scroll, the fade out timer is never started.
> >
> > This CL fixes the issue by moving the overflow requirement for overlay
> > scrollbars into ComputeScrollbarExistence so that we account for it no
> > matter the point at which we recompute scrollbar existence. We also
> > refactor ComputeScrollbarExistence to make clear the distinction between
> > overflow dependent and independent computations.
> >
> > A considered alternative was to always create the scrollbar but ensure
> > its enabled state is correctly set. This would have unified the
> > lifetimes of overlay and non-overlay scrollbars. However, it requires we
> > prevent creating composited layers for scrollbars that are overlay but
> > have no overflow since the compositor fades all of a scroller's
> > scrollbars together so such scrollbars would become visible if the other
> > axis is scrollable. We couldn't use the enabled state for this because
> > it is flipped every time the scrollbars fade in/out so that would cause
> > layers to constanty be created/destroyed so we'd have to introduce more
> > complexity to compositing decisions.
> >
> > This change caused three tests to break (note: unit tests generally use
> > mock overlay scrollbars):
> >
> > RepaintScrollableAreaLayersInMainThreadScrolling:
> >
> >   A style change like this does not require invalidating paint on
> >   overlay scrollbars' compositing layers since the compositor can deal
> >   with resizes on its own.  However, this was causing an invalidation
> >   because of the bug fixed here: the style change adds a horizontal
> >   scrollbar and the layout removes it. Because it isn't composited, it
> >   causes the whole PaintLayer to be invalidated[1]. This causes the
> >   scrollbar layer to also need a repaint[2]. Since we no longer cause
> >   this creation and destruction of the horizontal scrollbar, we don't
> >   get this invalidation.
> >
> >   This test now resizes the overflow so that the vertical scrollbar
> >   needs to be disabled which does cause an invalidation.
> >
> > [1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc?l=2845&rcl=33a9a3956034a765124f034a8f7d86f19c9c17e6
> > [2] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc?l=2099&rcl=01e24ce806680bf99abf117e77d30d9566c72121
> >
> > TraverseNonCompositingDescendantsInPaintOrder:
> >
> >   The source of the failure here was that the |overflow:scroll| div no
> >   longer has overlay scrollbars after its style is updated (but before
> >   it does layout). When it performs PaintLayer::UpdateSelfPaintingLayer
> >   it will no longer be self painting[3]. Since it no longer has a self
> >   painting layer it will be added to the parent's visual overflow. This
> >   larger rect means that during overlap testing, the scroller's sibling
> >   (#stacked-child-of-composited-non-stacking-context) and its parent
> >   will overlap and so the sibling will require compositing. Finally,
> >   because it is now composited, it is now a paint invalidation container
> >   and so it won't be invalidated from the container element[5].
> >
> >   The fix here is to simply add overflow to the scrolling div so that it
> >   continues to cause a self painting layer in [3].
> >
> > [3] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/paint_layer.cc?l=3054&rcl=fa18fef1b31a878fed89e2a543c3be0af2f57623
> > [4] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/layout_box.cc?l=5780&rcl=0650ec2415a0f2583db48d11c6d8b6a31ff448c4
> > [5] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/object_paint_invalidator.cc?l=70&rcl=d9d7df43f1718c900afc470371b382c8dca085b6
> >
> > mock-scrollbars.html:
> >
> >   This was failing only on the legacy LayoutNG disabled bot so I didn't
> >   dig too deeply into it. The failure here was that the inner box would
> >   layout as if the parent had non-overlay scrollbars. I believe this is
> >   because the mock scrollbars were being enabled at load, after layout
> >   was already completed. I think this used to work because the scrollbar
> >   was being added and then removed which would cause a layout
> >   invalidation which no longer happens, thus we need to ensure mock
> >   scrollbars are enabled before we produce the first layout.
> >
> > Bug: 1039501
> > Change-Id: Ic68522a4fdba8ffaa2c5ef508736c9a94eac7682
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2119933
> > Commit-Queue: David Bokan <bokan@chromium.org>
> > Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#754808}
>
> TBR=wangxianzhu@chromium.org,bokan@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 1039501
> Change-Id: Ifa1851da28d080686d7444db56ea660638f469d6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2140310
> Reviewed-by: David Bokan <bokan@chromium.org>
> Reviewed-by: Stephen Chenney <schenney@chromium.org>
> Commit-Queue: David Bokan <bokan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#757192}

TBR=wangxianzhu@chromium.org,bokan@chromium.org,schenney@chromium.org

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

Bug: 1039501,1068351,1069574
Change-Id: I5c272b3a2f32bd2fab09bfaa8b0895ffc3f4261c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2146451Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758414}
      c4f8ded9
    • David Bokan's avatar
      Fix text fragment for user activation · 6ba752a6
      David Bokan authored 
      For security reasons, text fragments must only be activated when
navigated with a user gesture. However, browser initiated navigations
(e.g. user typing in the omnibox, bookmarks) don't have the user gesture
bit set despite being initiated by the user (see discussion in
https://crrev.com/c/2132673 for details). Because of this limitation,
text fragment code explicitly checked if the navigation was browser
initiated, assuming that such navigations are always user activated.

However, history navigations are a special case. They're intentionally
considered to be browser initiated, even if they originate from renderer
script (e.g. `history.back()`). This meant that our check above would
allow script to use the history API to activate a text fragment without
a user gesture.

This CL explicitly forbids activating a text fragment if the navigation
is of history type. This is a trivial change (in terms of UX) because a
history navigation will restore the scroll position to where the user
left off so the text fragment scroll is already clobbered. This change
prevents a transient scroll that will be undone.

Note: we had an explicit test for this case that failed to catch the
failure. The reason was that the test was checking that the fragment
wasn't activated by checking that the scroll offset after a navigation
is 0. However, the text fragment's scroll would be clobbered (assuming
by history scroll restoration) so this check would erroneously pass. We
fix it in this CL by using a scroll listener so that we can tell a
scroll occurred even if it is later restored.

Bug: 1042986
Change-Id: Ia0ad9a8adcda2250603e6a7dd2b386193be2a6e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135407Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarNick Burris <nburris@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758413}
      6ba752a6
    • David Bokan's avatar
      Ensure blink_unittests run with mock scrollbars · 6e02a18d
      David Bokan authored 
      https://crrev.com/48614c8 added a DCHECK in Page so that we ensure all
code can access a ScrollbarTheme. This is because unit tests on Android
run without a platform theme engine. If a MockScrollbarTheme isn't
installed, any execution that requests a scrollbar theme will crash. A
scrollbar theme is essential to style and layout so this should always
be available.

However, this caused tests to break, revealing that some tests are
running without a mock theme. It's unclear why those tests are flakily
passing some runs and why the CL cleared the trybots in the first place;
the issue is 100% reproducible locally.

This CL fixes all blink unit tests to run with a mock theme. We default
a DummyPageHolder to use a MockOverlayScrollbar theme and manually
install it on other tests that don't use this class. A few tests create
multiple DummyPageHolders, these required a small fix to ensure the old
one is destructed before the new one is created so that we maintain
proper nesting of ScopedMockOverlayScrollbars.

One test in ImageDocumentTest required updated expectations as the test
now runs with overlay scrollbars, changing the required expected offset.

Bug: 1068595
Change-Id: I034c768e4b353dd0082ec5d568a0de073a28413b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2141213
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758412}
      6e02a18d
    • Jeroen Dhollander's avatar
      Revert "Uprev LibAssistant to 1.46s" · 8ce67c80
      Jeroen Dhollander authored 
      This reverts commit e3f29527.

Reason for revert: Rolldep of internal changes failed

Original change's description:
> Uprev LibAssistant to 1.46s
> 
> The current LibAssistant branch (1.42c) is no longer maintained,
> and also does not contain the latest features.
> 
>    * This will only be landed after M83 branches
>    * This depends on the internal changes to land first
> 
> Note: 
> Bug: b/152777569
> Change-Id: Ic34b6ca3ba1d793c719ebb58ab9766e943fc5ec6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134617
> Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
> Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
> Reviewed-by: Tao Wu <wutao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#758408}

TBR=xiaohuic@chromium.org,wutao@chromium.org,jeroendh@google.com

Change-Id: I022550bcb93f4201a63aa2b28f64f6ccb32e5005
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b/152777569
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2146450Reviewed-by: default avatarJeroen Dhollander <jeroendh@google.com>
Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
Cr-Commit-Position: refs/heads/master@{#758411}
      8ce67c80