Commit 0e39eff5 authored by Peter Kasting's avatar Peter Kasting Committed by Commit Bot

Refactor DownloadItemView state transitions, part 1.

The ultimate goal here is to eliminate duplication of code and work and
make the actual effects of state transitions obvious.

This CL combines the various TransitionTo...(), Show...(), Clear...(),
and SetMode() functions into a single UpdateMode() function and does
some initial basic combining of shared code.  It makes the following two
assumptions:

* The item does not transition from one warning state to the other, or
  from one mixed content state to the other; it transitions into or out
  of one of these states from an unrelated state (most commonly
  kNormal).  This seems true from my reading of the various model-side
  state transitions, and the existing code would not have properly
  handled such state changes anyway (e.g. |mode_| would never have been
  updated).

  Making this assumption lets us do conversions akin to this:

  OLD: if (is_mixed_content(mode) && !is_mixed_content(mode_)) { ... }
  NEW: if (is_mixed_content(mode)) { ... }

* It's undesirable to trigger LoadIcon()'s functionality again when the
  file path hasn't actually changed.  While only one call chain actually
  checked for this, it seems correct in principle in all cases (such
  repeated calls would just be extra work).

  Making this assumption lets us put that check in LoadIcon(), and then
  call it unconditionally at the end of UpdateMode() instead of in many
  other places.

The UpdateMode() function here is far too sprawling, but even making
fairly few actual logic changes, this CL already pushes the bounds of
what a reviewer can verify is still correct.  Further refactoring to
turn this into something shorter and more comprehensible will come in a
subsequent patch.

Bug: none
Change-Id: I6b0e6dfcfef5c1057c4e59e593adcee27e537950
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2296271
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarMin Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788857}
parent 05309ecd
...@@ -128,6 +128,12 @@ class DownloadItemView : public views::View, ...@@ -128,6 +128,12 @@ class DownloadItemView : public views::View,
// The space on the right side of the dangerous download label. // The space on the right side of the dangerous download label.
static constexpr int kLabelPadding = 8; static constexpr int kLabelPadding = 8;
// Returns the mode that best reflects the current model state.
Mode GetDesiredMode() const;
// Sets the current mode to |mode| and updates UI appropriately.
void UpdateMode(Mode mode);
void OpenDownload(); void OpenDownload();
// Submits the downloaded file to the safebrowsing download feedback service. // Submits the downloaded file to the safebrowsing download feedback service.
...@@ -142,7 +148,6 @@ class DownloadItemView : public views::View, ...@@ -142,7 +148,6 @@ class DownloadItemView : public views::View,
void DrawIcon(gfx::Canvas* canvas); void DrawIcon(gfx::Canvas* canvas);
void LoadIcon(); void LoadIcon();
void LoadIconIfItemPathChanged();
// Update the button colors based on the current theme. // Update the button colors based on the current theme.
void UpdateColorsFromTheme(); void UpdateColorsFromTheme();
...@@ -157,39 +162,6 @@ class DownloadItemView : public views::View, ...@@ -157,39 +162,6 @@ class DownloadItemView : public views::View,
// Sets the state and triggers a repaint. // Sets the state and triggers a repaint.
void SetDropdownState(State new_state); void SetDropdownState(State new_state);
void SetMode(Mode mode);
// Starts showing the normal mode dialog, clearing the existing dialog.
void TransitionToNormalMode();
// Starts showing the mixed content dialog, clearing the existing dialog.
void TransitionToMixedContentDialog();
// Reverts from mixed content modes to normal download mode.
void ClearMixedContentDialog();
// Starts displaying the mixed content download warning.
void ShowMixedContentDialog();
// Starts showing the warning dialog, clearing the existing dialog.
void TransitionToWarningDialog();
// Reverts from dangerous mode to normal download mode.
void ClearWarningDialog();
// Starts displaying the dangerous download warning or the malicious download
// warning.
void ShowWarningDialog();
// Starts showing the deep scanning dialog, clearing the existing dialog.
void TransitionToDeepScanningDialog();
// Reverts from deep scanning mode to normal download mode.
void ClearDeepScanningDialog();
// Starts displaying the deep scanning warning.
void ShowDeepScanningDialog();
// Returns the current warning icon (should only be called when the view is // Returns the current warning icon (should only be called when the view is
// actually showing a warning). // actually showing a warning).
gfx::ImageSkia GetWarningIcon(); gfx::ImageSkia GetWarningIcon();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment