New issue
Advanced search Search tips

Issue 704817 link

Starred by 2 users
Status: Fixed
Owner:
flackr@chromium.org
Closed: Nov 2017
Cc:
smcgruer@chromium.org
chrishtr@chromium.org
vmp...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Unanchored sticky position causes scroll on main and repaints on scroll

Project Member Reported by flackr@chromium.org, Mar 24 2017 
If a sticky position element has no anchor edges (i.e. top, bottom, left, right are all auto) then we should not add it to viewport constrained objects or compute any constraints for it, or repaint on scroll as it will behave exactly like a position relative element.

I.e. see http://output.jsbin.com/zikaqeqopa/quiet

Comment 1 by schenney@chromium.org, Mar 24 2017

Labels: BugSource-Team PaintTeamTriaged-20170324
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 10 2017 

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

commit 06e9b5a6a5649e4d842f40ad667554c58dc26c29
Author: flackr <flackr@chromium.org>
Date: Mon Apr 10 17:51:46 2017

Only create sticky position constraints for constrained sticky position.

If sticky position is unconstrained we don't need to create constraints for it,
or add it to viewport constrained objects (causing scroll on main), or repaint
on scroll.

BUG= 704817 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/06e9b5a6a5649e4d842f40ad667554c58dc26c29/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/06e9b5a6a5649e4d842f40ad667554c58dc26c29/third_party/WebKit/Source/core/frame/FrameViewTest.cpp
[modify] https://crrev.com/06e9b5a6a5649e4d842f40ad667554c58dc26c29/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
[modify] https://crrev.com/06e9b5a6a5649e4d842f40ad667554c58dc26c29/third_party/WebKit/Source/core/layout/LayoutBoxModelObjectTest.cpp
[modify] https://crrev.com/06e9b5a6a5649e4d842f40ad667554c58dc26c29/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
[modify] https://crrev.com/06e9b5a6a5649e4d842f40ad667554c58dc26c29/third_party/WebKit/Source/core/layout/compositing/CompositingInputsUpdater.cpp
[modify] https://crrev.com/06e9b5a6a5649e4d842f40ad667554c58dc26c29/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp
[modify] https://crrev.com/06e9b5a6a5649e4d842f40ad667554c58dc26c29/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/06e9b5a6a5649e4d842f40ad667554c58dc26c29/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
[modify] https://crrev.com/06e9b5a6a5649e4d842f40ad667554c58dc26c29/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/06e9b5a6a5649e4d842f40ad667554c58dc26c29/third_party/WebKit/Source/core/style/ComputedStyle.h

Comment 3 by flackr@chromium.org, Apr 10 2017

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 10 2017 

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

commit e0f87e573b0b8cba4b2d739a8c5a86c0f448d513
Author: hanxi <hanxi@chromium.org>
Date: Mon Apr 10 21:33:56 2017

Revert of Only create sticky position constraints for constrained sticky position. (patchset #4 id:60001 of https://codereview.chromium.org/2769353002/ )

Reason for revert:
It causes telemetry_perf_unittests failing on chromium.android/Android N5X Swarm Builder.

 crbug.com/710142 .

Original issue's description:
> Only create sticky position constraints for constrained sticky position.
>
> If sticky position is unconstrained we don't need to create constraints for it,
> or add it to viewport constrained objects (causing scroll on main), or repaint
> on scroll.
>
> BUG= 704817 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
>
> Review-Url: https://codereview.chromium.org/2769353002
> Cr-Commit-Position: refs/heads/master@{#463316}
> Committed: https://chromium.googlesource.com/chromium/src/+/06e9b5a6a5649e4d842f40ad667554c58dc26c29

TBR=chrishtr@chromium.org,flackr@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 704817 

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

[modify] https://crrev.com/e0f87e573b0b8cba4b2d739a8c5a86c0f448d513/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/e0f87e573b0b8cba4b2d739a8c5a86c0f448d513/third_party/WebKit/Source/core/frame/FrameViewTest.cpp
[modify] https://crrev.com/e0f87e573b0b8cba4b2d739a8c5a86c0f448d513/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
[modify] https://crrev.com/e0f87e573b0b8cba4b2d739a8c5a86c0f448d513/third_party/WebKit/Source/core/layout/LayoutBoxModelObjectTest.cpp
[modify] https://crrev.com/e0f87e573b0b8cba4b2d739a8c5a86c0f448d513/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
[modify] https://crrev.com/e0f87e573b0b8cba4b2d739a8c5a86c0f448d513/third_party/WebKit/Source/core/layout/compositing/CompositingInputsUpdater.cpp
[modify] https://crrev.com/e0f87e573b0b8cba4b2d739a8c5a86c0f448d513/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp
[modify] https://crrev.com/e0f87e573b0b8cba4b2d739a8c5a86c0f448d513/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/e0f87e573b0b8cba4b2d739a8c5a86c0f448d513/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
[modify] https://crrev.com/e0f87e573b0b8cba4b2d739a8c5a86c0f448d513/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/e0f87e573b0b8cba4b2d739a8c5a86c0f448d513/third_party/WebKit/Source/core/style/ComputedStyle.h

Comment 5 by flackr@chromium.org, Apr 11 2017

Status: Started (was: Fixed)

Comment 6 by flackr@chromium.org, Apr 11 2017 

Note this was reverted due to  issue 710142 , the change somehow caused the following failure:

telemetry_perf_unittests (benchmarks.system_health_smoke_test.SystemHealthBenchmarkSmokeTest.system_health.memory_mobile.background:news:nytimes) failing on chromium.android/Android N5X Swarm Builder

Builders failed on: 
- Android N5X Swarm Builder: 
  https://uberchromegw.corp.google.com/i/chromium.android/builders/Android%20N5X%20Swarm%20Builder/builds/11304

The print stack is:
(INFO) 2017-04-10 12:11:31,320 pid=12226  cmd_helper._ValidateAndLogCommand:161  [host]> /b/swarm_slave/w/ir/third_party/catapult/devil/bin/deps/linux2/x86_64/bin/adb devices
(INFO) 2017-04-10 12:11:31,329 pid=12226  cmd_helper._ValidateAndLogCommand:161  [host]> /b/swarm_slave/w/ir/third_party/catapult/devil/bin/deps/linux2/x86_64/bin/adb devices
[1/1] benchmarks.system_health_smoke_test.SystemHealthBenchmarkSmokeTest.system_health.memory_mobile.background:news:nytimes failed unexpectedly 34.4877s:
  [ RUN      ] background:news:nytimes@{'case': 'background', 'group': 'news'}
  Traceback (most recent call last):
    File "/b/swarm_slave/w/ir/third_party/catapult/telemetry/telemetry/internal/story_runner.py", line 89, in _RunStoryAndProcessErrorIfNeeded
      state.RunStory(results)
    File "/b/swarm_slave/w/ir/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py", line 75, in traced_function
      return func(*args, **kwargs)
    File "/b/swarm_slave/w/ir/third_party/catapult/telemetry/telemetry/page/shared_page_state.py", line 299, in RunStory
      self._current_page.Run(self)
    File "/b/swarm_slave/w/ir/third_party/catapult/telemetry/telemetry/page/__init__.py", line 112, in Run
      self.RunPageInteractions(action_runner)
    File "/b/swarm_slave/w/ir/tools/perf/page_sets/system_health/system_health_story.py", line 121, in RunPageInteractions
      self._DidLoadDocument(action_runner)
    File "/b/swarm_slave/w/ir/tools/perf/page_sets/system_health/background_stories.py", line 73, in _DidLoadDocument
      action_runner.TapElement(selector='.nytd-player-poster')
    File "/b/swarm_slave/w/ir/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py", line 75, in traced_function
      return func(*args, **kwargs)
    File "/b/swarm_slave/w/ir/third_party/catapult/telemetry/telemetry/internal/actions/action_runner.py", line 296, in TapElement
      selector=selector, text=text, element_function=element_function))
    File "/b/swarm_slave/w/ir/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py", line 75, in traced_function
      return func(*args, **kwargs)
    File "/b/swarm_slave/w/ir/third_party/catapult/telemetry/telemetry/internal/actions/action_runner.py", line 56, in _RunAction
      action.RunAction(self._tab)
    File "/b/swarm_slave/w/ir/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py", line 75, in traced_function
      return func(*args, **kwargs)
    File "/b/swarm_slave/w/ir/third_party/catapult/telemetry/telemetry/internal/actions/tap.py", line 68, in RunAction
      element_function=self.element_function)
    File "/b/swarm_slave/w/ir/third_party/catapult/telemetry/telemetry/internal/actions/page_action.py", line 127, in EvaluateCallbackWithElement
      return tab.EvaluateJavaScript(code)
    File "/b/swarm_slave/w/ir/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py", line 75, in traced_function
      return func(*args, **kwargs)
    File "/b/swarm_slave/w/ir/third_party/catapult/telemetry/telemetry/internal/browser/web_contents.py", line 167, in EvaluateJavaScript
      return self._inspector_backend.EvaluateJavaScript(*args, **kwargs)
    File "/b/swarm_slave/w/ir/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py", line 75, in traced_function
      return func(*args, **kwargs)
    File "/b/swarm_slave/w/ir/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py", line 37, in inner
      return func(inspector_backend, *args, **kwargs)
    File "/b/swarm_slave/w/ir/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py", line 244, in EvaluateJavaScript
      return self._runtime.Evaluate(expression, context_id, timeout)
    File "/b/swarm_slave/w/ir/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_runtime.py", line 54, in Evaluate
      description=details.get('exception', {}).get('description'))
  EvaluateException: UncaughtError:
  Error: Tap position is off-screen
      at TapAction.start (<anonymous>:59:13)
      at callback (<anonymous>:9:30)
      at <anonymous>:17:16
      at <anonymous>:18:9
  
  [  FAILED  ] background:news:nytimes@{'case': 'background', 'group': 'news'} (26374 ms)
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 21 2017 

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

commit 4405f51a4b20efd0344f524c91256014e53b56a5
Author: Robert Flack <flackr@chromium.org>
Date: Wed Jun 21 01:56:45 2017

Reland "Only create sticky position constraints for constrained sticky position.

This relands patchset #4 id:60001 of https://codereview.chromium.org/2769353002/.

Bug:  704817 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I737948ee6b81ec69a6f3ad778344b26423284554
Reviewed-on: https://chromium-review.googlesource.com/541636
Reviewed-by: Chris harrelson <chrishtr@chromium.org>
Commit-Queue: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481082}
[modify] https://crrev.com/4405f51a4b20efd0344f524c91256014e53b56a5/third_party/WebKit/Source/core/frame/LocalFrameView.cpp
[modify] https://crrev.com/4405f51a4b20efd0344f524c91256014e53b56a5/third_party/WebKit/Source/core/frame/LocalFrameViewTest.cpp
[modify] https://crrev.com/4405f51a4b20efd0344f524c91256014e53b56a5/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
[modify] https://crrev.com/4405f51a4b20efd0344f524c91256014e53b56a5/third_party/WebKit/Source/core/layout/LayoutBoxModelObjectTest.cpp
[modify] https://crrev.com/4405f51a4b20efd0344f524c91256014e53b56a5/third_party/WebKit/Source/core/layout/ScrollAnchor.cpp
[modify] https://crrev.com/4405f51a4b20efd0344f524c91256014e53b56a5/third_party/WebKit/Source/core/layout/ScrollAnchorTest.cpp
[modify] https://crrev.com/4405f51a4b20efd0344f524c91256014e53b56a5/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
[modify] https://crrev.com/4405f51a4b20efd0344f524c91256014e53b56a5/third_party/WebKit/Source/core/layout/compositing/CompositingInputsUpdater.cpp
[modify] https://crrev.com/4405f51a4b20efd0344f524c91256014e53b56a5/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp
[modify] https://crrev.com/4405f51a4b20efd0344f524c91256014e53b56a5/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/4405f51a4b20efd0344f524c91256014e53b56a5/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
[modify] https://crrev.com/4405f51a4b20efd0344f524c91256014e53b56a5/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/4405f51a4b20efd0344f524c91256014e53b56a5/third_party/WebKit/Source/core/style/ComputedStyle.h

Comment 8 by flackr@chromium.org, Nov 20 2017

Status: Fixed (was: Started)
►

Sign in to add a comment