Create content::WebContentsDestroyedWatcher, use it in many tests.

content/public/test/browser_test_utils.h

 // Copyright (c) 2012 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.
 
 #ifndef CONTENT_PUBLIC_TEST_BROWSER_TEST_UTILS_H_
 #define CONTENT_PUBLIC_TEST_BROWSER_TEST_UTILS_H_
 
 #include <queue>
 #include <string>
 #include <vector>
@@ skipping 205 matching lines @@
 
   std::vector<string16> expected_titles_;
   scoped_refptr<MessageLoopRunner> message_loop_runner_;
 
   // The most recently observed expected title, if any.
   string16 observed_title_;
 
   DISALLOW_COPY_AND_ASSIGN(TitleWatcher);
 };
 
+// Watches a WebContents and blocks until it is destroyed.
+class WebContentsDestroyedWatcher : public WebContentsObserver {

[jam, 2013/10/15 16:50:52]

i'm curious why this API isn't just a method? i.e.

CONTENT_EXPORT void WaitForWebContentsDestroyed(WebContents* web_contents)?

[Avi (use Gerrit), 2013/10/15 16:57:24]

Er... yeah. Good point, I hadn't thought about simplifying it that much.

[Avi (use Gerrit), 2013/10/15 20:16:32]

Actually, no, that's not a good design. That just begs for this to be flaky.

The flow should be:
- Start watching a WebContents
- Cause some action that will make it go away
- Wait for the destruction

There are two cases here.

One is that the test starts waiting for the destruction, and then the
destruction happens. Sure, if that were to be the only case, then yes,
WaitForWebContentsDestroyed would make sense.

But the other case is that the web contents gets destroyed before the waiting.
If watching the WebContents and waiting for it are all rolled up into one
function, then watching it comes waaaay to late and the test will hang.

This split between watching the WebContents and waiting for it to die is a Good
Thing.

[jam, 2013/10/15 20:47:15]

Yes I understand why we use that pattern in other places. I'm wondering about
this specific case of waiting for web contents destruction. Looking at the usage
of this method, it seems that none of them could happen before the Wait is
called.

[jam, 2013/10/15 20:48:04]

also, in this situation, if the WC is destroyed before Wait is called, it would
be very trivial to see this because instead of the usual timeout, it would be a
crash when calling the Observe method on WC

+ public:
+  explicit WebContentsDestroyedWatcher(WebContents* web_contents);
+  virtual ~WebContentsDestroyedWatcher();
+
+  // Waits until the WebContents is destroyed.
+  void Wait();
+
+ private:
+  // Overridden WebContentsObserver methods.
+  virtual void WebContentsDestroyed(WebContents* web_contents) OVERRIDE;
+
+  scoped_refptr<MessageLoopRunner> message_loop_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebContentsDestroyedWatcher);
+};
+
 // Watches for responses from the DOMAutomationController and keeps them in a
 // queue. Useful for waiting for a message to be received.
 class DOMMessageQueue : public NotificationObserver {
  public:
   // Constructs a DOMMessageQueue and begins listening for messages from the
   // DOMAutomationController. Do not construct this until the browser has
   // started.
   DOMMessageQueue();
   virtual ~DOMMessageQueue();
 
@@ skipping 14 matching lines @@
   std::queue<std::string> message_queue_;
   bool waiting_for_message_;
   scoped_refptr<MessageLoopRunner> message_loop_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(DOMMessageQueue);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_TEST_BROWSER_TEST_UTILS_H_