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

content/public/test/browser_test_utils.cc

Index: content/public/test/browser_test_utils.cc
diff --git a/content/public/test/browser_test_utils.cc b/content/public/test/browser_test_utils.cc
index 0864b5a2ae4d89bf797a839f21ec40f8e937e24a..8e4f9d955de28a2b99635002b2aa0591b1f5d5de 100644
--- a/content/public/test/browser_test_utils.cc
+++ b/content/public/test/browser_test_utils.cc
@@ -935,7 +935,7 @@ RenderFrameHost* FrameMatchingPredicate(
   std::set<RenderFrameHost*> frame_set;
   web_contents->ForEachFrame(
       base::Bind(&AddToSetIfFrameMatchesPredicate, &frame_set, predicate));
-  DCHECK_EQ(1U, frame_set.size());
+  EXPECT_EQ(1U, frame_set.size());

[sky, 2017/05/01 21:14:09]

One comment here. This should really be an ASSERT. The reason for that is with
an EXPECT, even if the predicate is false execution continues. So, when a
failure happens 939 is executed. If frame_set is empty, then 939 crashes. I
recommend the following, remove EXPECT and use a conditional, e.g.:

return !frame_size.size().empty() ? *frame_set.begin() : nullptr;

And then change callers to have the assert,
e.g
frame_host = FrameMatchingPredicate(...);
ASSERT_TRUE(frame_host);

[Łukasz Anforowicz, 2017/05/01 21:42:31]

Good catch - I should have been more careful about changing the DCHECK_EQ to
EXPECT_EQ.
This would change the test behavior - the tests would no longer catch the case
when a test case unexpectedly sees 2 frames with a matching predicate.

Having the callers ASSERT won't work for non-void functions.  Maybe we should
just go back to a DCHECK inside FrameMatchesPredicate?  WDYT?

PS. We could also modify FrameMatchingPredicate to take an extra parameter
|expected_number_of_matching_frames| that defaults to 1 (so preserving existing
behavior) and 1) keep EXPECT_EQ and 2) have null-dereference change you
proposed.  Doing things this way seems unnecessarily complicated and doesn't
really make FrameMatchingPredicate more versatile (for example the caller in
content_subresource_filter_driver_factory_unittest.cc just wants the first
maching frame [and doesn't care if there is 1, 2 or 6 matching frames]).

[sky, 2017/05/01 23:33:33]

How about changing the return to:
return frame_set.size() == 1 ? *frame_set.begin() : nullptr;
?

[Łukasz Anforowicz, 2017/05/02 19:55:48]

Done.

   return *frame_set.begin();
 }