Do not reinvent the wheel / use content APIs to find a frame by name.

chrome/browser/ui/views/drag_and_drop_interactive_uitest.cc

Index: chrome/browser/ui/views/drag_and_drop_interactive_uitest.cc
diff --git a/chrome/browser/ui/views/drag_and_drop_interactive_uitest.cc b/chrome/browser/ui/views/drag_and_drop_interactive_uitest.cc
index 3b051d1e4770ffbddbd9f7ec241fa3be5e780211..fb73c92faf8fbfbf097beb6bf8929f78893b1760 100644
--- a/chrome/browser/ui/views/drag_and_drop_interactive_uitest.cc
+++ b/chrome/browser/ui/views/drag_and_drop_interactive_uitest.cc
@@ -702,21 +702,8 @@ class DragAndDropBrowserTest : public InProcessBrowserTest,
   }
 
   content::RenderFrameHost* GetFrameByName(const std::string& name_to_find) {
-    content::RenderFrameHost* result = nullptr;
-    for (content::RenderFrameHost* rfh : web_contents()->GetAllFrames()) {
-      if (rfh->GetFrameName() == name_to_find) {
-        if (result) {
-          ADD_FAILURE() << "More than one frame named "
-                        << "'" << name_to_find << "'";

[Łukasz Anforowicz, 2017/04/28 22:52:09]

This ADD_FAILURE will be replaced by a DCHECK from
content::FrameMatchingPredicate.  Such change seems fine as discussed with
nick@.

[sky, 2017/05/01 16:50:30]

Remember that a DCHECK in tests causes a crash. Wouldn't it be better for this
code to ADD_FAILURE or EXPECT_TRUE?

[Łukasz Anforowicz, 2017/05/01 18:11:29]

Done.  Thanks for raising this point again - I previously thought that maybe the
dependency on gtest is undesirable (from content::FrameMatchingPredicate). but I
see that other code in content/public/test/browser_test_utils.cc already uses
gtest assertsions.  I agree that gtest assertion is nicer for tests than a
DCHECK.  I see that //content/test:test_support is |testonly|, so enforcing
FrameMatchingPredicate contract via test assertions seems reasonable (i.e. I
think it is okay to assume that FrameMatchingPredicate is only called from
within a gtest test).

-          return nullptr;
-        }
-        result = rfh;
-      }
-    }
-
-    EXPECT_TRUE(result) << "Couldn't find a frame named "
-                        << "'" << name_to_find << "'";

[Łukasz Anforowicz, 2017/04/28 22:52:09]

This EXPECT_TRUE will be replaced by a DCHECK from
content::FrameMatchingPredicate.  Such change seems fine as discussed with
nick@.

-    return result;
+    return content::FrameMatchingPredicate(
+        web_contents(), base::Bind(&content::FrameMatchesName, name_to_find));
   }
 
   void AssertTestPageIsLoaded() {