Fix clipboard nudge related crash

Crash can be triggered from login screen when any paste is performed and
ClipboardNudgeController::OnClipboardDataRead is called. So add a check
to see that the clipboard history is enabled in the current mode.

Also possible is that ClipboardNudgeController::ShouldShowNudge() is
passed a null PrefService*. So add a check for this being null, as well
as some checks for null dictionaries.

Bug: 1150168
Change-Id: I79e407f3896408af7a44aacf31f15d290c2629c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2546085
Commit-Queue: Matthew Mourgos <mmourgos@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828866}

ash/clipboard/clipboard_history.cc

@@ -6,8 +6,6 @@
 
 #include "ash/clipboard/clipboard_history_util.h"
 #include "ash/clipboard/clipboard_nudge_controller.h"
-#include "ash/session/session_controller_impl.h"
-#include "ash/shell.h"
 #include "base/stl_util.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "ui/base/clipboard/clipboard_monitor.h"
@@ -71,7 +69,7 @@ void ClipboardHistory::RemoveItemForId(const base::UnguessableToken& id) {
 }
 
 void ClipboardHistory::OnClipboardDataChanged() {
-  if (!IsEnabledInCurrentMode())
+  if (!ClipboardHistoryUtil::IsEnabledInCurrentMode())
     return;
 
   // TODO(newcomer): Prevent Clipboard from recording metrics when pausing
@@ -108,20 +106,6 @@ void ClipboardHistory::OnClipboardDataChanged() {
                      commit_data_weak_factory_.GetWeakPtr(), *clipboard_data));
 }
 
-bool ClipboardHistory::IsEnabledInCurrentMode() const {
-  switch (Shell::Get()->session_controller()->login_status()) {
-    case LoginStatus::NOT_LOGGED_IN:
-    case LoginStatus::LOCKED:
-    case LoginStatus::KIOSK_APP:
-    case LoginStatus::PUBLIC:
-      return false;
-    case LoginStatus::USER:
-    case LoginStatus::GUEST:
-    case LoginStatus::SUPERVISED:
-      return true;
-  }
-}
-
 void ClipboardHistory::MaybeCommitData(ui::ClipboardData data) {
   if (!ClipboardHistoryUtil::IsSupported(data))
     return;

ash/clipboard/clipboard_history.h

@@ -70,9 +70,6 @@ class ASH_EXPORT ClipboardHistory : public ui::ClipboardObserver {
   // ClipboardMonitor:
   void OnClipboardDataChanged() override;
 
-  // Returns whether the clipboard history is enabled for the current user mode.
-  bool IsEnabledInCurrentMode() const;
-
  private:
   // Adds `data` to the `history_list_` if it's supported. If `data` is not
   // supported by clipboard history, this method no-ops.

ash/clipboard/clipboard_history_controller_impl.cc

@@ -258,7 +258,7 @@ void ClipboardHistoryControllerImpl::ShowMenu(
 
 bool ClipboardHistoryControllerImpl::CanShowMenu() const {
   return !clipboard_history_->IsEmpty() &&
-         clipboard_history_->IsEnabledInCurrentMode();
+         ClipboardHistoryUtil::IsEnabledInCurrentMode();
 }
 
 void ClipboardHistoryControllerImpl::OnClipboardHistoryCleared() {

ash/clipboard/clipboard_history_util.cc

@@ -8,6 +8,8 @@
 
 #include "ash/clipboard/clipboard_history_item.h"
 #include "ash/metrics/histogram_macros.h"
+#include "ash/session/session_controller_impl.h"
+#include "ash/shell.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "ui/base/clipboard/clipboard_data.h"
@@ -109,5 +111,19 @@ bool IsSupported(const ui::ClipboardData& data) {
   return true;
 }
 
+bool IsEnabledInCurrentMode() {
+  switch (Shell::Get()->session_controller()->login_status()) {
+    case LoginStatus::NOT_LOGGED_IN:
+    case LoginStatus::LOCKED:
+    case LoginStatus::KIOSK_APP:
+    case LoginStatus::PUBLIC:
+      return false;
+    case LoginStatus::USER:
+    case LoginStatus::GUEST:
+    case LoginStatus::SUPERVISED:
+      return true;
+  }
+}
+
 }  // namespace ClipboardHistoryUtil
 }  // namespace ash

ash/clipboard/clipboard_history_util.h

@@ -89,6 +89,9 @@ ASH_EXPORT base::string16 GetFileSystemSources(const ui::ClipboardData& data);
 // Returns true if `data` is supported by clipboard history.
 ASH_EXPORT bool IsSupported(const ui::ClipboardData& data);
 
+// Returns whether the clipboard history is enabled for the current user mode.
+ASH_EXPORT bool IsEnabledInCurrentMode();
+
 }  // namespace ClipboardHistoryUtil
 }  // namespace ash
 

ash/clipboard/clipboard_nudge_controller.cc

@@ -5,6 +5,7 @@
 #include "ash/clipboard/clipboard_nudge_controller.h"
 
 #include "ash/clipboard/clipboard_history_item.h"
+#include "ash/clipboard/clipboard_history_util.h"
 #include "ash/clipboard/clipboard_nudge.h"
 #include "ash/clipboard/clipboard_nudge_constants.h"
 #include "ash/public/cpp/ash_pref_names.h"
@@ -111,8 +112,10 @@ void ClipboardNudgeController::OnClipboardHistoryItemAdded(
 void ClipboardNudgeController::OnClipboardDataRead() {
   PrefService* prefs =
       Shell::Get()->session_controller()->GetLastActiveUserPrefService();
-  if (!ShouldShowNudge(prefs))
+  if (!ClipboardHistoryUtil::IsEnabledInCurrentMode() ||
+      !ShouldShowNudge(prefs)) {
     return;
+  }
 
   switch (clipboard_state_) {
     case ClipboardState::kFirstCopy:
@@ -255,22 +258,26 @@ const ClipboardState& ClipboardNudgeController::GetClipboardStateForTesting() {
 }
 
 int ClipboardNudgeController::GetShownCount(PrefService* prefs) {
-  base::Optional<int> nudge_shown_count =
-      prefs->GetDictionary(prefs::kMultipasteNudges)->FindIntPath(kShownCount);
-  return nudge_shown_count.value_or(0);
+  const base::DictionaryValue* dictionary =
+      prefs->GetDictionary(prefs::kMultipasteNudges);
+  if (!dictionary)
+    return 0;
+  return dictionary->FindIntPath(kShownCount).value_or(0);
 }
 
 base::Time ClipboardNudgeController::GetLastShownTime(PrefService* prefs) {
-  const base::Value* last_shown_time_val =
-      prefs->GetDictionary(prefs::kMultipasteNudges)->FindPath(kLastTimeShown);
-  if (!last_shown_time_val)
+  const base::DictionaryValue* dictionary =
+      prefs->GetDictionary(prefs::kMultipasteNudges);
+  if (!dictionary)
     return base::Time();
   base::Optional<base::Time> last_shown_time =
-      *util::ValueToTime(last_shown_time_val);
+      util::ValueToTime(dictionary->FindPath(kLastTimeShown));
   return last_shown_time.value_or(base::Time());
 }
 
 bool ClipboardNudgeController::ShouldShowNudge(PrefService* prefs) {
+  if (!prefs)
+    return false;
   int nudge_shown_count = GetShownCount(prefs);
   base::Time last_shown_time = GetLastShownTime(prefs);
   // We should not show more nudges after hitting the limit.