Upstream native crash handling changes for Android.

base/android/build_info.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 BASE_ANDROID_BUILD_INFO_H_
+#define BASE_ANDROID_BUILD_INFO_H_
+
+#include <jni.h>
+#include <string>

[Mark Mentovai, 2012/04/05 14:28:03]

Separate C system headers from C++ system headers with a blank line.

[carlosvaldivia, 2012/04/05 18:35:56]

Done.

+
+namespace base {
+namespace android {
+
+

[Mark Mentovai, 2012/04/05 14:28:03]

Get rid of this extra blank line and the one at line 56.

[carlosvaldivia, 2012/04/05 18:35:56]

Done.

+// BuildInfo is a singleton class that stores android build and device
+// information.
+class BuildInfo {
+ public:
+
+  // Static factory method for getting the singleton BuildInfo instance.

[Mark Mentovai, 2012/04/05 14:28:03]

Observe the proper ordering within a class, per
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Declar...
.

Your static function should follow the destructor.

[carlosvaldivia, 2012/04/05 18:35:56]

Done.

+  static const BuildInfo* const GetInstance();

[Mark Mentovai, 2012/04/05 14:28:03]

Say somewhere that this leaks all of its data and explain why it’s OK.

[Mark Mentovai, 2012/04/05 14:28:03]

Consider using a Singleton (base/memory/singleton.h) instead of implementing
this custom stuff.

[carlosvaldivia, 2012/04/05 18:35:56]

Done.

[carlosvaldivia, 2012/04/05 18:35:56]

I'll get to that along with the linux/posix_breakpad rename after I respond to
these comments.

On 2012/04/05 14:28:03, Mark Mentovai wrote:

+
+  ~BuildInfo() {};

[Mark Mentovai, 2012/04/05 14:28:03]

The semicolon; is unnecessary.

[carlosvaldivia, 2012/04/05 18:35:56]

Done.

+
+  // We use const char* instead of std::strings because we need to have these

[Mark Mentovai, 2012/04/05 14:28:03]

Don”t write “we” in comments.

[carlosvaldivia, 2012/04/05 18:35:56]

Done.

+  // values available even if the process is in a crash state. Sadly
+  // std::string.c_str() doesn't guarantee that memory won't be allocated when
+  // it is called.
+  const char* device;

[Mark Mentovai, 2012/04/05 14:28:03]

Data members should always be private.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Access...

[carlosvaldivia, 2012/04/05 18:35:56]

Done.

+  const char* model;
+  const char* brand;
+  const char* android_build_id;
+  const char* android_build_fp;
+  const char* package_version_code;
+  const char* package_version_name;
+
+ private:
+  static BuildInfo* instance_;

[Mark Mentovai, 2012/04/05 14:28:03]

private + static + used directly only in one file? Sounds like you should take
it out of the class altogether and just put it in the .cc file that uses it.

[carlosvaldivia, 2012/04/05 18:35:56]

It is a jni wrapper. There is java code that uses it as well.

On 2012/04/05 14:28:03, Mark Mentovai wrote:

+
+  BuildInfo(
+      const char* device,
+      const char* model,
+      const char* brand,
+      const char* android_build_id,
+      const char* android_build_fp,
+      const char* package_version_code,
+      const char* package_version_name);
+

[Mark Mentovai, 2012/04/05 14:28:03]

DISALLOW_COPY_AND_ASSIGN?

[carlosvaldivia, 2012/04/05 18:35:56]

Done.

+};
+
+bool RegisterBuildInfo(JNIEnv* env);
+
+}  // namespace android
+}  // namespace base
+
+
+#endif  // BASE_ANDROID_BUILD_INFO_H_