Introduce VisiblePosition::isValid

third_party/WebKit/Source/core/editing/VisiblePosition.cpp

Index: third_party/WebKit/Source/core/editing/VisiblePosition.cpp
diff --git a/third_party/WebKit/Source/core/editing/VisiblePosition.cpp b/third_party/WebKit/Source/core/editing/VisiblePosition.cpp
index a9187c09d098b1deaa903fcad6f959e989ae41a8..2e012f6fded35540c5933a6f8a19a222c4ca322b 100644
--- a/third_party/WebKit/Source/core/editing/VisiblePosition.cpp
+++ b/third_party/WebKit/Source/core/editing/VisiblePosition.cpp
@@ -45,12 +45,18 @@ using namespace HTMLNames;
 
 template <typename Strategy>
 VisiblePositionTemplate<Strategy>::VisiblePositionTemplate()
+#if DCHECK_IS_ON()
+    : m_domTreeVersion(0)

[yosin_UTC9, 2016/09/20 13:55:51]

We also need to track |Document::styleTreeVersion()|, e.g. display:none changes
visible position.
See "Selection.{cpp,h}" in http://crrev.com/1958093002

[Xiaocheng, 2016/09/21 06:31:49]

I can't find |Document::styleTreeVersion()| in the code base...

I searched "TreeVersion" but found only dom tree version:

https://cs.chromium.org/search/?q=TreeVersion+case:yes&p=2&sq=package:chromiu...

+#endif
 {
 }
 
 template <typename Strategy>
 VisiblePositionTemplate<Strategy>::VisiblePositionTemplate(const PositionWithAffinityTemplate<Strategy>& positionWithAffinity)
     : m_positionWithAffinity(positionWithAffinity)
+#if DCHECK_IS_ON()
+    , m_domTreeVersion(positionWithAffinity.position().document()->domTreeVersion())
+#endif
 {
 }
 
@@ -214,6 +220,19 @@ void VisiblePositionTemplate<Strategy>::showTreeForThis() const
 
 #endif
 
+template <typename Strategy>
+bool VisiblePositionTemplate<Strategy>::isValid() const
+{
+#if DCHECK_IS_ON()
+    if (isNull())
+        return true;
+    Document& document = *m_positionWithAffinity.position().document();
+    return m_domTreeVersion == document.domTreeVersion() && !document.needsLayoutTreeUpdate();

[yosin_UTC9, 2016/09/20 13:55:51]

Fumm... If we check |Document::needsLayoutTreeUpdate()| here, do we need to
check dom tree version and style tree version?
Is checking |Document::needsLayoutTreeUpdate()| enough?

[Xiaocheng, 2016/09/21 06:31:49]

I think both dom tree version and |Document::needsLayoutTreeUpdate| need to be
checked. We don't want to see an invalid VP becoming valid again after layout
update.

We also have to check style tree version, as we don't want to see a VP
invalidated due to style change becoming valid again. Unfortunately, style tree
version is not available in the code base (or at least I didn't find it).

I'm wondering if we have both dom and style tree versions, do we still need to
check |Document::needsLayoutTreeUpdate|? Or perhaps it simply doesn't harm to
have more checks...

+#else
+    return true;
+#endif
+}
+
 template class CORE_TEMPLATE_EXPORT VisiblePositionTemplate<EditingStrategy>;
 template class CORE_TEMPLATE_EXPORT VisiblePositionTemplate<EditingInFlatTreeStrategy>;