Support custom icon in search result.

chrome/browser/chromeos/launcher_search_provider/error_reporter.h

Index: chrome/browser/chromeos/launcher_search_provider/error_reporter.h
diff --git a/chrome/browser/chromeos/launcher_search_provider/error_reporter.h b/chrome/browser/chromeos/launcher_search_provider/error_reporter.h
new file mode 100644
index 0000000000000000000000000000000000000000..98b1e14cf0cc690e7132f838fee4df7828e84c54
--- /dev/null
+++ b/chrome/browser/chromeos/launcher_search_provider/error_reporter.h
@@ -0,0 +1,41 @@
+// 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_LAUNCHER_SEARCH_PROVIDER_ERROR_REPORTER_H_
+#define CHROME_BROWSER_CHROMEOS_LAUNCHER_SEARCH_PROVIDER_ERROR_REPORTER_H_
+
+#include <string>
+
+#include "base/memory/ref_counted.h"
+#include "ipc/ipc_sender.h"
+
+namespace chromeos {
+namespace launcher_search_provider {
+
+// A utility class which sends error message to developer console.
+class ErrorReporter : public base::RefCounted<ErrorReporter> {

[satorux1, 2015/05/03 23:51:24]

Why does it have to be a ref counted object? Ref counting should generally be
avoided. IIRC, this wasn't a ref counted class when I originally saw the patch.

[Matt Giuca, 2015/05/05 04:25:30]

There was a use-after-free in the older patch set.

Can you make it by-value and copyable instead? ErrorReporter is essentially just
a struct that can be freely copied around (since it doesn't own any objects).
Much simpler than refcounted.

(See the earlier discussion in launcher_search_provider.cc.)

[yawano, 2015/05/07 02:27:17]

This wasn't ref counted object before.

The reason that I changed this to ref counted object is for testing. We cannot
simply copy the value since we inherits this class in unit test (the value will
be copied as original class, right?). Also we need to reference same instance
from test code and production code to check the error message.

I'm going to find a way not to use ref counted object here.

[Matt Giuca, 2015/05/07 03:24:03]

I see. Right, you can't directly copy an object that has a subclass (or you will
be calling the base class copy constructor with the subclass, and that will
throw away any fields of the subclass). You also can't directly store an object
of this type; it must be pointed to.

The usual solution to this is to use scoped_ptr everywhere (using .Pass() where
possible to avoid copying), and have a method "virtual scoped_ptr<ErrorReporter>
Duplicate() const" that creates a proper copy of the object, overriding it in
each subclass to correctly duplicate.

You might want the subclass (in the test) to internally have a
linked_ptr<std::string> (rather than directly having a std::string), so that if
the MockErrorReporter gets duplicated, all of the copies share the same string.
(It may sound complicated, but I prefer this because it constrains the reference
counting to the tests which is the reason we needed it in the first place.)

[yawano, 2015/05/07 06:24:55]

Done. Thank you for the idea.

+ public:
+  ErrorReporter(IPC::Sender* sender, const int routing_id);
+
+  // Shows |message| as warning in the developer console of the extension.
+  virtual void Warn(const std::string& message);
+
+ protected:
+  virtual ~ErrorReporter();
+
+ private:
+  friend class base::RefCounted<ErrorReporter>;
+
+  // Not owned.
+  IPC::Sender* sender_;
+
+  const int routing_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(ErrorReporter);
+};
+
+}  // namespace launcher_search_provider
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_LAUNCHER_SEARCH_PROVIDER_ERROR_REPORTER_H_