Offer user to send feedback from profile error dialog

sql/connection.h

Index: sql/connection.h
diff --git a/sql/connection.h b/sql/connection.h
index 12e7041c9fd2fa10f7bec8c04b182c14c5d302ad..288625e5569f2bba7cbe63bb93c09b6e2dbbbdfa 100644
--- a/sql/connection.h
+++ b/sql/connection.h
@@ -20,6 +20,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
+#include "sql/db_diagnostic_map.h"

[michaeln, 2016/07/09 03:00:38]

probably overkill to add a new file for using a std::map<> alias, unless there's
a compelling reason not i think you can put it in this

[Scott Hess - ex-Googler, 2016/07/10 05:16:52]

Honestly, I'm not sure I see why this is in sql/ or sql:: at all, nor can I see
why it's a map.  Is there some long-term plan which justifies any of those
decisions?  Why not just have the various clients pass along a string which they
keep appending key:value data to and be done with it?

I'll admit, figuring out where to put the shared code to generate the diagnostic
info is a tough one.  As a sql/OWNER member, I'd rather that such code wasn't
arbitrarily pushed into here simply because you couldn't figure out a better
place to put it.  But I can see how that might happen.  Having happened, I'd
rather it were as simple as possible, and not uncommented and named as if it's a
general-purpose method which just anyone should be using.

[afakhry, 2016/07/11 16:47:46]

The reasons behind my decision to put it there were the following:
- I was following the same convention in "sql/init_status.h" which is only there
to include an enum used by the clients of sqlite rather than sqlite itself.
   - I could have just added the DatabaseDiagnosticMap there in
"sql/init_status.h" , but it felt even more awkward, since DatabaseDiagnosticMap
is actually used by sql::Connection inside sql/ and the file name will be
misleading.

- I thought of declaring DatabaseDiagnosticMap in connection.h, but I wasn't
sure if I wanted clients to include "connection.h" whenever they needed this
map.

The reasons behind choosing a map:
- It keeps the keys sorted, which is a nice thing to have if we decide to expand
the list of diagnostic information we add, or integrate this data with the
feedback App system information window, which contains sorted key-value pair of
information.
- It makes the code shorter and cleaner; just map["key"] = value;
- It is also easier for clients to query for certain diagnostics key (might be
useful in the future).

That being said, I'm open for alternatives/suggestions if they're found to be
better than what's mentioned above.

[Scott Hess - ex-Googler, 2016/07/13 01:18:46]

sql/init_status.h is also really client code which really probably shouldn't be
in sql/.
This seems completely arbitrary, why should the lexicographic ordering of the
keys be the best way to sort them?  As opposed to some other ordering like "The
order things ran in"?
result.append("key: ").append(value).append("\n");

I don't think making diagnostic code shorter and cleaner should be a primary
factor.  I mean, we shouldn't make it long and horrible, but I don't think
you're making an order-of-magnitude improvement in this case.
I think that you shouldn't design a general-purpose diagnostics-querying engine
in an ad-hoc manner.  If you want to build such a system, you should probably
put up a design doc and post it around to see what people want to dump into it.
What are the downsides of a std::string blob?  Not the "Maybe someday someone
might want to do something different" downsides, what are the downsides for the
problem currently in front of you?

[afakhry, 2016/07/13 19:57:59]

Nothing other than the reason I stated above. I'm now using string as suggested.

 #include "sql/sql_export.h"
 
 struct sqlite3;
@@ -239,6 +240,8 @@ class SQL_EXPORT Connection {
   // without error and results in a single "ok" value.
   bool QuickIntegrityCheck() WARN_UNUSED_RESULT;
 
+  sql::DatabaseDiagnosticMap GetDiagnosticMap();
+
   // Initialization ------------------------------------------------------------
 
   // Initializes the SQL connection for the given file, returning true if the