Handle anonymous layout objects correctly in SnapCoordinator

Handle anonymous layout objects correctly in SnapCoordinator

This is done by not assume that |node()| is always available.

BUG=637423
Committed: https://crrev.com/4add15dab9bc86608da3158f25a57582e1f86dd0
Cr-Commit-Position: refs/heads/master@{#431881}

[majidvp, 2016-09-13 17:31:02]

The CQ bit was checked by majidvp@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-13 17:31:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 18:48:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-13 18:48:57]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[majidvp, 2016-09-14 18:35:37]

The CQ bit was checked by majidvp@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-14 18:36:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[majidvp, 2016-09-14 18:36:33]

Description was changed from

==========
Handle anonymous layout objects correctly in SnapCoordinator

Do not assume that |node()| is always available.

BUG=637423
==========

to

==========
Handle anonymous layout objects correctly in SnapCoordinator

This is done by not assume that |node()| is always available.

BUG=637423
==========

[majidvp, 2016-09-14 18:36:49]

majidvp@chromium.org changed reviewers:
+ skobes@chromium.org

[commit-bot: I haz the power, 2016-09-14 20:45:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-14 20:45:30]

Dry run: This issue passed the CQ dry run.

[skobes, 2016-09-16 01:11:16]

Should this have a test?

[majidvp, 2016-11-01 14:53:12]

On 2016/09/16 01:11:16, skobes wrote:
> Should this have a test?

I had a hard time coming up with a good test.
I don't know a good way to create, access and modify the style of 
an anonymous layout object to inject a snap point to trigger this. 
Any pointers?


The original fuzz test case was achieving this indirectly via
video.webkitRequestFullScreen()
which somehow creates an anonymous layout object that inherits 
some of its parent's style but I don't think that is appropriate
to use here.

I have manually confirmed that the fix addresses the fuzz 
finding and I am not sure if it is even worth spending more time
on creating a test case for the more general case.

[skobes, 2016-11-02 21:40:11]

ok, lgtm

[majidvp, 2016-11-03 14:11:49]

The CQ bit was checked by majidvp@chromium.org

[commit-bot: I haz the power, 2016-11-03 14:12:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-03 15:41:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-03 15:41:25]

Failed to apply patch for
third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp:
While running git apply --index -3 -p1;
  error: patch failed:
third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp:33
  Falling back to three-way merge...
  Applied patch to
'third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp' with
conflicts.
  U third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp

Patch:       third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp
Index: third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp
diff --git a/third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp
b/third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp
index
f3412809610faa1962b4e1e599f733fc2e7f24c0..478cbb7ea41968d6faaf8c8848e0091945b0f283
100644
--- a/third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp
+++ b/third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp
@@ -33,14 +33,14 @@ static LayoutBox* findSnapContainer(const LayoutBox&
snapArea)
     // According to the new spec
https://drafts.csswg.org/css-scroll-snap/#snap-model
     // "Snap positions must only affect the nearest ancestor (on the element’s
     // containing block chain) scroll container".
-    Element* viewportDefiningElement =
snapArea.node()->document().viewportDefiningElement();
+    Element* viewportDefiningElement =
snapArea.document().viewportDefiningElement();
     LayoutBox* box = snapArea.containingBlock();
     while (box && !box->hasOverflowClip() && !box->isLayoutView() &&
box->node() != viewportDefiningElement)
         box = box->containingBlock();
 
     // If we reach to viewportDefiningElement then we dispatch to viewport
-    if (box->node() == viewportDefiningElement)
-        return snapArea.node()->document().layoutView();
+    if (box && box->node() == viewportDefiningElement)
+        return snapArea.document().layoutView();
 
     return box;
 }

[majidvp, 2016-11-10 21:31:35]

The CQ bit was checked by majidvp@chromium.org

[majidvp, 2016-11-10 21:31:35]

The patchset sent to the CQ was uploaded after l-g-t-m from skobes@chromium.org
Link to the patchset: https://codereview.chromium.org/2332383002/#ps20001
(title: "rebase")

[commit-bot: I haz the power, 2016-11-10 21:32:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-11 02:16:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-11 02:16:12]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[majidvp, 2016-11-14 15:19:01]

The CQ bit was checked by majidvp@chromium.org

[commit-bot: I haz the power, 2016-11-14 15:19:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-14 19:10:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-14 19:10:31]

Failed to apply patch for
third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp:
While running git apply --index -p1;
  error: patch failed:
third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp:30
  error: third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp:
patch does not apply

Patch:       third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp
Index: third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp
diff --git a/third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp
b/third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp
index
7eb7f145dc8471019a597414e47fe64e844328db..ad27eae93f1cd9aa888c8602c6fb324511d27984
100644
--- a/third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp
+++ b/third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp
@@ -30,15 +30,15 @@ static LayoutBox* findSnapContainer(const LayoutBox&
snapArea) {
   // "Snap positions must only affect the nearest ancestor (on the element’s
   // containing block chain) scroll container".
   Element* viewportDefiningElement =
-      snapArea.node()->document().viewportDefiningElement();
+      snapArea.document().viewportDefiningElement();
   LayoutBox* box = snapArea.containingBlock();
   while (box && !box->hasOverflowClip() && !box->isLayoutView() &&
          box->node() != viewportDefiningElement)
     box = box->containingBlock();
 
   // If we reach to viewportDefiningElement then we dispatch to viewport
-  if (box->node() == viewportDefiningElement)
-    return snapArea.node()->document().layoutView();
+  if (box && box->node() == viewportDefiningElement)
+    return snapArea.document().layoutView();
 
   return box;
 }

[commit-bot: I haz the power, 2016-11-14 19:34:41]

Description was changed from

==========
Handle anonymous layout objects correctly in SnapCoordinator

This is done by not assume that |node()| is always available.

BUG=637423
==========

to

==========
Handle anonymous layout objects correctly in SnapCoordinator

This is done by not assume that |node()| is always available.

BUG=637423

Committed: https://crrev.com/4add15dab9bc86608da3158f25a57582e1f86dd0
Cr-Commit-Position: refs/heads/master@{#431881}
==========

[commit-bot: I haz the power, 2016-11-14 19:34:41]

Patchset 2 (id:??) landed as
https://crrev.com/4add15dab9bc86608da3158f25a57582e1f86dd0
Cr-Commit-Position: refs/heads/master@{#431881}