Open the learn more page when user clicks a notification of malicious download

chrome/browser/download/notification/download_notification_item.cc

Index: chrome/browser/download/notification/download_notification_item.cc
diff --git a/chrome/browser/download/notification/download_notification_item.cc b/chrome/browser/download/notification/download_notification_item.cc
index 2e9438b5600690ed144436fab192f9cad4c56239..921fc8a7dcc76c732d788a75b8ae3f7290d753eb 100644
--- a/chrome/browser/download/notification/download_notification_item.cc
+++ b/chrome/browser/download/notification/download_notification_item.cc
@@ -119,6 +119,11 @@ void DownloadNotificationItem::OnNotificationClick() {
     } else {
       item_->SetOpenWhenComplete(!item_->GetOpenWhenComplete());  // Toggle

[asanka, 2015/05/14 22:07:42]

Does this also cause the notification to close automatically when the download
is opened?

[yoshiki, 2015/05/18 16:51:59]

No, the notification doesn't close. When the download finishes, the file opens
and the notification center closes if it's opened, but the notification leaves
in the notification center.

     }
+#if defined(FULL_SAFE_BROWSING)
+  } else if (item_->IsDangerous()) {

[asanka, 2015/05/14 22:07:42]

At this point, I think it would be readable to break this into blocks with early
returns. A chain of if() else if() ... blocks with heterogeneous conditionals
become very difficult to reason about as they grow larger.

For example, the fact that IsDangerous() reaches here and won't satisfy the
openable_ condition above can only be deemed by inspecting how 'openable_' is
set, and then backtracking to figure out that GetPossibleActions() won't return
OPEN... for a dangerous download. Nobody reading this code is going to put in
that much effort unless they are specifically looking for a bug in this logic.

Ideally we could find a way to centralize the concept of 'possible actions' into
GetPossibleActions(), where it also determines a default action. Then
OnNotificationClick() would just invoke the default action.

Or if you want to keep the default action determination here, let's organize
this method in a manner similar to GetPossibleActions(). I.e. something like:

OnNotificationClick() {
#if defined...
  if (item_->IsDangerous()) {
     Open help center article;
     return;
  }
#endif

  if (openable_) {
    ...
    return;
  }

  if (item_->GetState() == INTERRUPTED or CANCELLED) {
    ...
    return;
  }

  CloseNotificationByUser();
}

[yoshiki, 2015/05/18 16:51:59]

Thanks, I refined the code of OnNotificationClick and GetPossibleActions
(renamed to GetExtraActions) to make them similar. 

GetPossibleActions was originally in DownloadCommands and moved to
DownloadNotificationItem, so some code didn't match the code here.

+    DownloadCommands(item_).ExecuteCommand(
+        DownloadCommands::LEARN_MORE_SCANNING);
+#endif
   } else if (item_->GetState() == content::DownloadItem::INTERRUPTED ||
              item_->GetState() == content::DownloadItem::CANCELLED) {
     GetBrowser()->OpenURL(content::OpenURLParams(