Refined DCHECK preventing multiple event forwarders in VA tree path

ui/android/view_android.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/android/view_android.h"
 
 #include <algorithm>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
@@ skipping 86 matching lines @@
   JNIEnv* env = base::android::AttachCurrentThread();
   delegate_ = JavaObjectWeakGlobalRef(env, delegate);
 }
 
 float ViewAndroid::GetDipScale() {
   return ui::GetScaleFactorForNativeView(this);
 }
 
 ScopedJavaLocalRef<jobject> ViewAndroid::GetEventForwarder() {
   if (!event_forwarder_) {
-    DCHECK(!ViewTreeHasEventForwarder(this))
-        << "Root of the ViewAndroid can have at most one handler.";
+    DCHECK(!RootPathHasEventForwarder(this) && !SubtreeHasEventForwarder(this))
+        << "The view tree path already has an event forwarder.";
     event_forwarder_.reset(new EventForwarder(this));
   }
   return event_forwarder_->GetJavaObject();
 }
 
 void ViewAndroid::AddChild(ViewAndroid* child) {
   DCHECK(child);
   DCHECK(std::find(children_.begin(), children_.end(), child) ==
          children_.end());
-  DCHECK(!SubtreeHasEventForwarder(child) || !ViewTreeHasEventForwarder(this))
-      << "Only one event handler is allowed.";
+  DCHECK(
+      (!RootPathHasEventForwarder(this) && !SubtreeHasEventForwarder(child)) ||
+      !has_event_forwarder())
+      << "Some view tree path will have more than one event forwarder "
+         "if the child is added.";
 
   // The new child goes to the top, which is the end of the list.
   children_.push_back(child);
   if (child->parent_)
     child->RemoveFromParent();
   child->parent_ = this;
 }
 
 // static
-bool ViewAndroid::ViewTreeHasEventForwarder(ViewAndroid* view) {
-  ViewAndroid* v = view;
-  do {
-    if (v->has_event_forwarder())
+bool ViewAndroid::RootPathHasEventForwarder(ViewAndroid* view) {
+  view = view->parent_;

[boliu, 2017/05/10 23:55:13]

start with view, not view->parent_, and then fix caller accordingly

[Jinsuk Kim, 2017/05/11 00:17:14]

Done.

+  while (view) {
+    if (view->has_event_forwarder())
       return true;
-    v = v->parent_;
-  } while (v);
-  return SubtreeHasEventForwarder(view);
+    view = view->parent_;
+  }
+
+  return false;
 }
 
 // static
 bool ViewAndroid::SubtreeHasEventForwarder(ViewAndroid* view) {
   if (view->has_event_forwarder())
     return true;
+
   for (auto* child : view->children_) {
     if (SubtreeHasEventForwarder(child))
       return true;
   }
   return false;
 }
 
 void ViewAndroid::MoveToFront(ViewAndroid* child) {
   DCHECK(child);
   auto it = std::find(children_.begin(), children_.end(), child);
@@ skipping 179 matching lines @@
         matched = bound.Contains(e->GetX(0), e->GetY(0));
       }
       if (matched && child->HitTest(send_to_client, *e))
         return true;
     }
   }
   return false;
 }
 
 }  // namespace ui