GitLab

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: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788857}