Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(128)

Unified Diff: ash/display/display_error_observer_chromeos.cc

Issue 1636153002: Give user ability to file a feedback report from the display error notification. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix errors Created 4 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: ash/display/display_error_observer_chromeos.cc
diff --git a/ash/display/display_error_observer_chromeos.cc b/ash/display/display_error_observer_chromeos.cc
index c08e6402da45394b01d650ecc0de98a094d06851..fe89db88f8a421f08ec936e11dd5c7e58501a258 100644
--- a/ash/display/display_error_observer_chromeos.cc
+++ b/ash/display/display_error_observer_chromeos.cc
@@ -4,9 +4,13 @@
#include "ash/display/display_error_observer_chromeos.h"
+#include <cinttypes>
#include <utility>
+#include "ash/new_window_delegate.h"
+#include "ash/shell.h"
#include "ash/system/system_notifier.h"
+#include "base/strings/stringprintf.h"
#include "grit/ash_resources.h"
#include "grit/ash_strings.h"
#include "ui/base/l10n/l10n_util.h"
@@ -23,6 +27,48 @@ namespace {
const char kDisplayErrorNotificationId[] = "chrome://settings/display/error";
+// A notification delegate that will start the feedback app when the notication
+// is clicked.
+class DisplayErrorNotificationDelegate
+ : public message_center::NotificationDelegate {
+ public:
+ DisplayErrorNotificationDelegate() {}
oshima 2016/01/27 09:18:14 = default. (this seems to be allowed now)
afakhry 2016/01/29 02:16:51 Done.
+
+ // message_center::NotificationDelegate:
+ bool HasClickedListener() override { return true; }
+
+ void Click() override {
+ ash::Shell::GetInstance()->new_window_delegate()->OpenFeedbackPage();
+ }
+
+ private:
+ // Private destructor since NotificationDelegate is ref-counted.
+ ~DisplayErrorNotificationDelegate() override {}
oshima 2016/01/27 09:18:14 ditto
afakhry 2016/01/29 02:16:51 Done.
+
+ DISALLOW_COPY_AND_ASSIGN(DisplayErrorNotificationDelegate);
+};
+
+// Returns an error message to be logged when there is a failure in changing
+// the display mode, so that it shows in the system logs when the user files
+// a feedback report.
+std::string CreateErrorLogMessage(
+ const ui::DisplayConfigurator::DisplayStateList& displays) {
+ std::string error_log_message(
+ "Failed to configure the following display(s):\n");
+ for (const auto& display : displays) {
+ base::StringAppendF(&error_log_message,
+ "\t- Display with ID = %" PRId64 ", and EDID = ",
+ display->display_id());
+
+ for (const auto& edid_byte : display->edid())
+ base::StringAppendF(&error_log_message, "%02X", edid_byte);
oshima 2016/01/27 09:18:14 base::HexEncode?
afakhry 2016/01/29 02:16:51 Done.
+
+ base::StringAppendF(&error_log_message, ".\n");
+ }
+
+ return error_log_message;
oshima 2016/01/27 09:18:14 can't you jus log it to the stream instead of crea
afakhry 2016/01/29 02:16:51 Done.
+}
+
} // namespace
DisplayErrorObserver::DisplayErrorObserver() {
@@ -34,6 +80,8 @@ DisplayErrorObserver::~DisplayErrorObserver() {
void DisplayErrorObserver::OnDisplayModeChangeFailed(
const ui::DisplayConfigurator::DisplayStateList& displays,
ui::MultipleDisplayState new_state) {
+ LOG(ERROR) << CreateErrorLogMessage(displays);
+
// Always remove the notification to make sure the notification appears
// as a popup in any situation.
message_center::MessageCenter::Get()->RemoveNotification(
@@ -53,7 +101,8 @@ void DisplayErrorObserver::OnDisplayModeChangeFailed(
GURL(),
message_center::NotifierId(message_center::NotifierId::SYSTEM_COMPONENT,
system_notifier::kNotifierDisplayError),
- message_center::RichNotificationData(), NULL));
+ message_center::RichNotificationData(),
+ new DisplayErrorNotificationDelegate));
message_center::MessageCenter::Get()->AddNotification(
std::move(notification));
}

Powered by Google App Engine
This is Rietveld 408576698