Kill renderers for bad password forms in --site-per-process.

components/password_manager/content/browser/bad_message.cc

 // Copyright 2015 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.
 
-#include "extensions/browser/bad_message.h"
+#include "components/password_manager/content/browser/bad_message.h"
 
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "content/public/browser/render_process_host.h"
 
-namespace extensions {
+namespace password_manager {
 namespace bad_message {
 
 void ReceivedBadMessage(content::RenderProcessHost* host,
                         BadMessageReason reason) {
-  LOG(ERROR) << "Terminating extension renderer for bad IPC message, reason "
-             << reason;
-  UMA_HISTOGRAM_ENUMERATION("Stability.BadMessageTerminated.Extensions", reason,
-                            BAD_MESSAGE_MAX);
+  LOG(ERROR)

[vabr (Chromium), 2015/07/08 08:26:27]

nit: Please make this a VLOG at least, to allow modular filtering. Moreover,
please make this even DVLOG, unless both of the following is true:
* it would be hard to distinguish this type of renderer crashes otherwise
* we are likely to need this log from users with non-debug builds

(See also http://dev.chromium.org/developers/coding-style#TOC-Logging.)

[lfg, 2015/07/08 15:31:47]

I believe this is one of the rare cases that we need to log this even on release
builds since the browser process has chosen to terminate the renderer. This is
not logged as a crash and it's otherwise impossible to tell the reason for the
renderer termination. This has been used in the past to track issues in
content/browser/bad_message.h.

Maybe nick@ has some other thoughts about this?

[vabr (Chromium), 2015/07/09 06:29:32]

Fair enough about the release builds, but still, please why not make it a
VLOG(1) instead of LOG(ERROR)? They should output at the same time without
VLOG-specific flags, but VLOGs can also be filtered for modules.

+      << "Terminating renderer for bad PasswordManager IPC message, reason "
+      << reason;
+  UMA_HISTOGRAM_ENUMERATION("Stability.BadMessageTerminated.PasswordManager",
+                            reason, BAD_MESSAGE_MAX + 1);

[ncarter (slow), 2015/07/07 22:18:32]

Do we really need the +1?

[lfg, 2015/07/08 15:31:47]

Yes, it was needed because histograms don't work when there's only a single
bucket (2 is the minimum). However, now that I've split the reasons, it's not
needed anymore.

   host->ShutdownForBadMessage();
 }
 
 }  // namespace bad_message
-}  // namespace extensions
+}  // namespace password_manager