Delete "shutdown without closing browsers" path

chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc

Index: chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
index ebaf3285b545b2a7197b0d7e518eb1c276ff26d7..dd08d269da31a9ae46852373694ddcf898f3af0f 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
@@ -995,12 +995,7 @@ void BookmarkBarView::BookmarkModelLoaded(BookmarkModel* model,
 }
 
 void BookmarkBarView::BookmarkModelBeingDeleted(BookmarkModel* model) {
-  // In normal shutdown The bookmark model should never be deleted before us.
-  // When X exits suddenly though, it can happen, This code exists
-  // to check for regressions in shutdown code and not crash.
-  if (!browser_shutdown::ShuttingDownWithoutClosingBrowsers())
-    NOTREACHED();
-
+  NOTREACHED();

[Albert Bodenhamer, 2014/03/23 01:38:36]

It doesn't make sense to have a NOTREACHED() followed by the cleanup code.

[oshima, 2014/03/23 04:07:35]

This method should not be called in normal path, and NOTREACHED was always valid
when "ShuttingDownWithoutClosingBrowesrs()" is false,
as you can see.

Since I'm removing the special path, NOTREACHED should remain and
the following code is a fallback path for release build.

[Albert Bodenhamer, 2014/03/24 02:22:42]

Isn't NOTREACHED basically a DCHECK? I thought the normal rule was not to handle
DCHECK failure cases. Either calling this code is an error (NOTREACHED) or we
should treat this as a rare, but valid, cleanup case.

I'm not going to hold up the CL any more over it. Just want to understand the
motivation.

   // Do minimal cleanup, presumably we'll be deleted shortly.
   model_->RemoveObserver(this);
   model_ = NULL;