Document/ShadowRoot split of pointerLockElement

third_party/WebKit/Source/core/dom/DocumentOrShadowRoot.h

Index: third_party/WebKit/Source/core/dom/DocumentOrShadowRoot.h
diff --git a/third_party/WebKit/Source/core/dom/DocumentOrShadowRoot.h b/third_party/WebKit/Source/core/dom/DocumentOrShadowRoot.h
index 11fbf32b7b9da7d7754da24cd0ee2058703e0a52..c4aed37e515d900221b7b3252bddce2a944c4195 100644
--- a/third_party/WebKit/Source/core/dom/DocumentOrShadowRoot.h
+++ b/third_party/WebKit/Source/core/dom/DocumentOrShadowRoot.h
@@ -7,6 +7,7 @@
 
 #include "core/dom/Document.h"
 #include "core/dom/shadow/ShadowRoot.h"
+#include "core/frame/UseCounter.h"
 
 namespace blink {
 
@@ -46,6 +47,38 @@ public:
     {
         return treeScope.elementsFromPoint(x, y);
     }
+
+    static Element* retargetPointerLockElement(TreeScope& scope, const Element* target)

[hayato, 2016/06/27 06:43:13]

Make a target a reference: const Element& target

[kochi, 2016/06/27 07:11:36]

Done.

+    {

[hayato, 2016/06/27 06:43:13]

This is not a spec compliant.

Please see the definition of the activeElement carefully. Strictly speaking, it
is not the result of retargeting.
There is one more additional check for activeElement:  
Thus, this function is actually like emulating the behavior of activeElement,
but not retargeting.

It would be better to add a test to find the difference between
*activeElement-like behavior* and *retargeting*.

e.g.

Put a pointer Locked element in an ancestor tree of a shadow tree.

[kochi, 2016/06/27 07:11:36]

This was intentional because if Shadow V1 hosts V0 shadow, and the real
locked element is in V0 shadow, V1 returns the element as it is.

I'd like to do the same for fullscreenElement, so I think now I'd better
have a design doc for this describing why (not just in the review comment).

I'll add a comment in this function, update the spec patch, and double-check
tests.

[kochi, 2016/06/28 06:34:06]

This function moved to TreeScope's method.

+        const Element* elementToReturn = target;
+        for (const Element* ancestor = target; ancestor; ancestor = ancestor->shadowHost()) {
+            if (ancestor->shadowRootIfV1())
+                elementToReturn = ancestor;
+            if (scope == ancestor->treeScope())
+                return const_cast<Element*>(elementToReturn);
+        }
+        return nullptr;
+    }
+
+    static Element* pointerLockElement(Document& document)
+    {
+        UseCounter::count(document, UseCounter::DocumentPointerLockElement);
+        const Element* target = document.pointerLockElement();

[hayato, 2016/06/27 06:43:13]

Return early if target is null.

[kochi, 2016/06/27 07:11:36]

Done.

+        if (target && target->isInV0ShadowTree()) {
+            UseCounter::count(document, UseCounter::DocumentPointerLockElementInV0Shadow);
+            return const_cast<Element*>(target);

[hayato, 2016/06/27 06:43:13]

Could you add a comment here so that we can remove this special handling in
confident in the future?

[kochi, 2016/06/28 06:34:06]

Done.

+        }
+        return retargetPointerLockElement(document, target);
+    }
+
+    static Element* pointerLockElement(ShadowRoot& shadowRoot)
+    {
+        if (!shadowRoot.isV1())

[hayato, 2016/06/27 06:43:13]

Could you add a comment here so that we can remove this special handling in
confident in the future?

[kochi, 2016/06/27 07:11:36]

Done.

[kochi, 2016/06/27 07:11:36]

Done.

+            return nullptr;
+        UseCounter::count(shadowRoot.document(), UseCounter::ShadowRootPointerLockElement);
+        const Element* target = shadowRoot.document().pointerLockElement();

[hayato, 2016/06/27 06:43:13]

Return early if target is null.

[kochi, 2016/06/27 07:11:36]

Done.

+        return retargetPointerLockElement(shadowRoot, target);
+    }
 };
 
 } // namespace blink