Try to improve comments and temp/param names.

Also makes one or two functions const.

Bug: none
Change-Id: I9a67f5f5839aef92468342c0dfcba36da864d4b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2348195
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797488}

chrome/browser/download/download_shelf.h

@@ -60,17 +60,15 @@ class DownloadShelf {
   // Closes the shelf.
   void Close();
 
-  // Hides the shelf. This closes the shelf if it is currently showing.
+  // Closes the shelf and prevents it from reopening until Unhide() is called.
   void Hide();
 
-  // Unhides the shelf. This will cause the shelf to be opened if it was open
-  // when it was hidden, or was shown while it was hidden.
+  // Allows the shelf to open after a previous call to Hide().  Opens the shelf
+  // if, had Hide() not been called, it would currently be open.
   void Unhide();
 
   Browser* browser() { return browser_; }
-
-  // Returns whether the download shelf is hidden.
-  bool is_hidden() { return is_hidden_; }
+  bool is_hidden() const { return is_hidden_; }
 
  protected:
   virtual void DoShowDownload(DownloadUIModel::DownloadUIModelPtr download) = 0;
@@ -86,7 +84,7 @@ class DownloadShelf {
   Profile* profile() { return profile_; }
 
  private:
-  // Show the download on the shelf immediately. Also displayes the download
+  // Shows the download on the shelf immediately. Also displays the download
   // started animation if necessary.
   void ShowDownload(DownloadUIModel::DownloadUIModelPtr download);
 

chrome/browser/download/test_download_shelf.h

@@ -21,7 +21,6 @@ class TestDownloadShelf : public DownloadShelf {
   bool IsShowing() const override;
   bool IsClosing() const override;
 
-  // Return |true| if a download was added to this shelf.
   bool did_add_download() const { return did_add_download_; }
 
  protected:

chrome/browser/ui/views/download/download_item_view.cc

@@ -461,9 +461,9 @@ void DownloadItemView::OnDownloadUpdated() {
     return;
   }
 
-  base::string16 new_tip = model_->GetTooltipText();
-  if (new_tip != tooltip_text_) {
-    tooltip_text_ = new_tip;
+  const base::string16 new_tooltip_text = model_->GetTooltipText();
+  if (new_tooltip_text != tooltip_text_) {
+    tooltip_text_ = new_tooltip_text;
     TooltipTextChanged();
   }
 }
@@ -508,10 +508,10 @@ void DownloadItemView::AnimationEnded(const gfx::Animation* animation) {
 }
 
 void DownloadItemView::MaybeSubmitDownloadToFeedbackService(
-    DownloadCommands::Command download_command) {
+    DownloadCommands::Command command) {
   if (!model_->ShouldAllowDownloadFeedback() ||
-      !SubmitDownloadToFeedbackService(download_command))
-    ExecuteCommand(download_command);
+      !SubmitDownloadToFeedbackService(command))
+    ExecuteCommand(command);
 }
 
 gfx::Size DownloadItemView::CalculatePreferredSize() const {
@@ -1183,22 +1183,21 @@ void DownloadItemView::OpenDownloadDuringAsyncScanning() {
 }
 
 bool DownloadItemView::SubmitDownloadToFeedbackService(
-    DownloadCommands::Command download_command) {
+    DownloadCommands::Command command) const {
 #if BUILDFLAG(FULL_SAFE_BROWSING)
   safe_browsing::SafeBrowsingService* sb_service =
       g_browser_process->safe_browsing_service();
   if (!sb_service)
     return false;
-  safe_browsing::DownloadProtectionService* download_protection_service =
+  safe_browsing::DownloadProtectionService* dp_service =
       sb_service->download_protection_service();
-  if (!download_protection_service)
+  if (!dp_service)
     return false;
   // TODO(shaktisahu): Enable feedback service for offline item.
   if (model_->download()) {
-    return download_protection_service->MaybeBeginFeedbackForDownload(
-        shelf_->browser()->profile(), model_->download(), download_command);
+    return dp_service->MaybeBeginFeedbackForDownload(
+        shelf_->browser()->profile(), model_->download(), command);
   }
-  // WARNING: |this| has been deleted!
   return true;
 #else
   NOTREACHED();

chrome/browser/ui/views/download/download_item_view.h

@@ -105,10 +105,9 @@ class DownloadItemView : public views::View,
   const DownloadUIModel* model() const { return model_.get(); }
 
   // Submits download to download feedback service if the user has approved and
-  // the download is suitable for submission, then applies |download_command|.
+  // the download is suitable for submission, then applies |command|.
   // If user hasn't seen SBER opt-in text before, show SBER opt-in dialog first.
-  void MaybeSubmitDownloadToFeedbackService(
-      DownloadCommands::Command download_command);
+  void MaybeSubmitDownloadToFeedbackService(DownloadCommands::Command command);
 
  protected:
   // views::View:
@@ -170,7 +169,7 @@ class DownloadItemView : public views::View,
   // buttons are given the same size).
   gfx::Size GetButtonSize() const;
 
-  // Returns the file name to report to user. It might be elided to fit into
+  // Returns the file name to report to the user. It might be elided to fit into
   // the text width. |label| dictates the default text style.
   base::string16 ElidedFilename(const views::StyledLabel& label) const;
 
@@ -202,10 +201,9 @@ class DownloadItemView : public views::View,
   void OpenDownloadDuringAsyncScanning();
 
   // Submits the downloaded file to the safebrowsing download feedback service.
-  // Applies |download_command| if submission succeeds. Returns whether
-  // submission was successful.
-  bool SubmitDownloadToFeedbackService(
-      DownloadCommands::Command download_command);
+  // Applies |command| if submission succeeds. Returns whether submission was
+  // successful.
+  bool SubmitDownloadToFeedbackService(DownloadCommands::Command command) const;
 
   // Forwards |command| to |commands_|; useful for callbacks.
   void ExecuteCommand(DownloadCommands::Command command);

chrome/browser/ui/views/download/download_shelf_view.cc

@@ -242,15 +242,15 @@ void DownloadShelfView::AnimationEnded(const gfx::Animation* animation) {
     }
   }
 
-  // If we had keyboard focus, calling SetVisible(false) causes keyboard focus
-  // to be completely lost. To prevent this, we focus another view: the web
-  // contents. TODO(collinbaker): https://crbug.com/846466 Fix
-  // AccessiblePaneView::SetVisible or FocusManager to make this unnecessary.
+  // Make the shelf non-visible.
+  //
+  // If we had keyboard focus, calling SetVisible(false) will cause keyboard
+  // focus to be completely lost. To prevent this, focus the web contents.
+  // TODO(crbug.com/846466): Fix AccessiblePaneView::SetVisible() or
+  // FocusManager to make this unnecessary.
   auto* focus_manager = GetFocusManager();
-  if (focus_manager && Contains(focus_manager->GetFocusedView())) {
+  if (focus_manager && Contains(focus_manager->GetFocusedView()))
     parent_->contents_web_view()->RequestFocus();
-  }
-
   SetVisible(false);
 }
 

chrome/browser/ui/views/download/download_shelf_view.h

@@ -95,6 +95,8 @@ class DownloadShelfView : public DownloadShelf,
 
   // The download views. These are also child Views, and deleted when
   // the DownloadShelfView is deleted.
+  // TODO(pkasting): Remove this in favor of making these the children of a
+  // nested view, so they can easily be laid out and iterated.
   std::vector<DownloadItemView*> download_views_;
 
   // Button for showing all downloads (chrome://downloads).