Add crash keys inside Mac's Widget::CloseAllSecondaryWidgets()

crbug/788271 is a mystery, crashing in objc_msgSend without triggering
any zombie warnings.

crbug/808318 points to a leaked WidgetObserver affecting all platforms,
but has no indication of what the observer is, or what window it observes.

Record class names and the window title in crash keys. The window title
is usually set by the framework, even if it is not displayed anywhere.

Bug: 788271, 808318
Change-Id: I09f9697bfca4e91c8f3b0e8dba125c1eb39cde4b
Reviewed-on: https://chromium-review.googlesource.com/896774
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534240}

ui/views/BUILD.gn

@@ -684,6 +684,7 @@ jumbo_component("views") {
 
   if (is_mac) {
     deps += [
+      "//components/crash/core/common",
       "//ui/accelerated_widget_mac",
       "//ui/events:dom_keycode_converter",
     ]

ui/views/DEPS

 include_rules = [
   "+cc/paint",
+  "+components/crash/core/common/crash_key.h",
   "+components/vector_icons",
   "+services/ui/public/interfaces",
   "+skia/ext",

ui/views/widget/native_widget_mac.mm

@@ -13,6 +13,7 @@
 #include "base/mac/scoped_nsobject.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "components/crash/core/common/crash_key.h"
 #import "ui/base/cocoa/constrained_window/constrained_window_animation.h"
 #import "ui/base/cocoa/window_size_constants.h"
 #include "ui/gfx/font_list.h"
@@ -635,11 +636,28 @@ NativeWidgetMacNSWindow* NativeWidgetMac::CreateNSWindow(
 
 // static
 void Widget::CloseAllSecondaryWidgets() {
-  // Create a copy of [NSApp windows] to increase every window's retain count.
-  // -[NSWindow dealloc] won't be invoked on any windows until this array goes
-  // out of scope.
-  base::scoped_nsobject<NSArray> starting_windows([[NSApp windows] copy]);
-  for (NSWindow* window in starting_windows.get()) {
+  NSArray* starting_windows = [NSApp windows];  // Creates an autoreleased copy.
+  for (NSWindow* window in starting_windows) {
+    // Crash keys for http://crbug.com/788271 and http://crbug.com/808318.
+    // Crashes suggest the window delegate may have become invalid, so record
+    // a separate key before sending messages to the delegate.
+    // TODO(tapted): Remove the delegate key. The window info key is probably
+    // useful to keep to help diagnose leaked observers crashing in Widget code.
+    static crash_reporter::CrashKeyString<256> window_info_key("windowInfo");
+    static crash_reporter::CrashKeyString<256> window_delegate_info_key(
+        "windowDelegateInfo");
+    std::string value = base::SysNSStringToUTF8(
+        [NSString stringWithFormat:@"Closing %@ (%@)", [window title],
+                                   [window className]]);
+    crash_reporter::ScopedCrashKeyString scopedWindowKey(&window_info_key,
+                                                         value);
+
+    NSObject* delegate = [window delegate];
+    value = base::SysNSStringToUTF8([NSString
+        stringWithFormat:@"NSWindow delegate class: %@", [delegate className]]);
+    crash_reporter::ScopedCrashKeyString scopedDelegateKey(
+        &window_delegate_info_key, value);
+
     Widget* widget = GetWidgetForNativeWindow(window);
     if (widget && widget->is_secondary_widget())
       [window close];

ui/views/widget/native_widget_mac_unittest.mm

@@ -760,6 +760,76 @@ TEST_F(NativeWidgetMacTest, NonWidgetParent) {
   EXPECT_EQ(0u, [[native_parent childWindows] count]);
 }
 
+// Tests that CloseAllSecondaryWidgets behaves in various configurations.
+TEST_F(NativeWidgetMacTest, CloseAllSecondaryWidgetsValidState) {
+  NSWindow* last_window = nil;
+  {
+    // First verify the behavior of CloseAllSecondaryWidgets in the normal case,
+    // and how [NSApp windows] changes in response to Widget closure.
+    base::mac::ScopedNSAutoreleasePool pool;
+    Widget* widget = CreateTopLevelPlatformWidget();
+    widget->Show();
+    TestWidgetObserver observer(widget);
+    last_window = widget->GetNativeWindow();
+    EXPECT_TRUE([[NSApp windows] containsObject:last_window]);
+    Widget::CloseAllSecondaryWidgets();
+    EXPECT_TRUE(observer.widget_closed());
+  }
+
+  {
+    // Calls to [NSApp windows] do autorelease, so ensure the pool empties.
+    base::mac::ScopedNSAutoreleasePool pool;
+
+    // [NSApp windows] updates inside dealloc, so the window should be gone.
+    EXPECT_FALSE([[NSApp windows] containsObject:last_window]);
+  }
+
+  {
+    // Repeat, but now retain a reference and close the window before
+    // CloseAllSecondaryWidgets().
+    base::mac::ScopedNSAutoreleasePool pool;
+    Widget* widget = CreateTopLevelPlatformWidget();
+    widget->Show();
+    TestWidgetObserver observer(widget);
+    last_window = [widget->GetNativeWindow() retain];
+    EXPECT_TRUE([[NSApp windows] containsObject:last_window]);
+    widget->CloseNow();
+    EXPECT_TRUE(observer.widget_closed());
+  }
+  {
+    base::mac::ScopedNSAutoreleasePool pool;
+    // Reference retained, so the window should still be present.
+    EXPECT_TRUE([[NSApp windows] containsObject:last_window]);
+  }
+
+  {
+    base::mac::ScopedNSAutoreleasePool pool;
+    Widget::CloseAllSecondaryWidgets();
+    [last_window release];
+  }
+  {
+    base::mac::ScopedNSAutoreleasePool pool;
+    EXPECT_FALSE([[NSApp windows] containsObject:last_window]);
+  }
+
+  // Repeat, with two Widgets. We can't control the order of window closure.
+  // If the parent is closed first, it should tear down the child while
+  // iterating over the windows. -[NSWindow close] will be sent to the child
+  // twice, but that should be fine.
+  Widget* parent = CreateTopLevelPlatformWidget();
+  Widget* child = CreateChildPlatformWidget(parent->GetNativeView());
+  parent->Show();
+  child->Show();
+  TestWidgetObserver parent_observer(parent);
+  TestWidgetObserver child_observer(child);
+
+  EXPECT_TRUE([[NSApp windows] containsObject:parent->GetNativeWindow()]);
+  EXPECT_TRUE([[NSApp windows] containsObject:child->GetNativeWindow()]);
+  Widget::CloseAllSecondaryWidgets();
+  EXPECT_TRUE(parent_observer.widget_closed());
+  EXPECT_TRUE(child_observer.widget_closed());
+}
+
 // Tests closing the last remaining NSWindow reference via -windowWillClose:.
 // This is a regression test for http://crbug.com/616701.
 TEST_F(NativeWidgetMacTest, NonWidgetParentLastReference) {