Setting CommandLabel 'IDS_DOWNLOAD_NOTIFICATION_LABEL_OPEN" for finished Download Notification.

chrome/browser/download/notification/download_item_notification.cc

 // 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.
 
 #include "chrome/browser/download/notification/download_item_notification.h"
 
 #include "base/files/file_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/download/download_crx_util.h"
@@ skipping 664 matching lines @@
 }
 
 base::string16 DownloadItemNotification::GetCommandLabel(
     DownloadCommands::Command command) const {
   int id = -1;
   switch (command) {
     case DownloadCommands::OPEN_WHEN_COMPLETE:
       if (item_ && !item_->IsDone())
         id = IDS_DOWNLOAD_STATUS_OPEN_WHEN_COMPLETE;
       else
-        id = IDS_DOWNLOAD_STATUS_OPEN_WHEN_COMPLETE;
+        id = IDS_DOWNLOAD_STATUS_OPEN;

[asanka, 2015/09/25 03:18:47]

yoshiki: Aren't these labels for the command buttons?

The text "Opening when complete" isn't grammatically correct for the open
button. The IDS_DOWNLOAD_STATUS_* strings are for indicating the status of the
download. If the user elects to open the download when it completes, then the
status says "Opening when complete" or "Opening when done". But that text isn't
suited for use as a command label.

A more appropriate string would be something closer to the
IDS_DOWNLOAD_MENU_OPEN_WHEN_COMPLETE and IDS_DOWNLOAD_MENU_OPEN.

[Deepak, 2015/09/25 16:09:33]

In DownloadShelfContextMenu::GetLabelForCommandId()
it is using IDS_DOWNLOAD_MENU_OPEN_WHEN_COMPLETE and IDS_DOWNLOAD_MENU_OPEN
But here all the labels have IDS_DOWNLOAD_MENU_* strings that is menu strings.
It appears that DownloadItemNotification::GetCommandLabel() is for non menu
items. So I have introduced IDS_DOWNLOAD_STATUS_OPEN, on similar lines.
This is as per my understanding. Please correct me if I misunderstood.
I would like to hear more on this from @yoshiki.
Thanks

[asanka, 2015/09/25 17:09:35]

_DOWNLOAD_STATUS_ is for download status strings. They shouldn't be used to
describe actions.

[Deepak, 2015/09/27 06:19:57]

I got your point. Changes done.
Thanks

       break;
     case DownloadCommands::PAUSE:
       // Only for non menu.
       id = IDS_DOWNLOAD_LINK_PAUSE;
       break;
     case DownloadCommands::RESUME:
       // Only for non menu.
       id = IDS_DOWNLOAD_LINK_RESUME;
       break;
     case DownloadCommands::SHOW_IN_FOLDER:
@@ skipping 190 matching lines @@
                                                     profile_id);
 
   message_center::NotificationList::Notifications visible_notifications =
       g_browser_process->message_center()->GetVisibleNotifications();
   for (const auto& notification : visible_notifications) {
     if (notification->id() == notification_id_in_message_center)
       return true;
   }
   return false;
 }