Fix reading ad-hoc settings with no formal Settings object

Summary:
Ref T4103. Some settings (like the collapsed/expanded state of the diff filetree) are currently ad-hoc. They weren't being read correctly.

Also, simplify the caching code a little bit.

Test Plan: Toggled filetree, reloaded page, got sticky behavior.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4103

Differential Revision: https://secure.phabricator.com/D16021

src/applications/people/storage/PhabricatorUser.php

@@ -505,13 +505,13 @@ final class PhabricatorUser
             ->setViewer($this)
             ->assertValidValue($value);
 
-          $this->settingCacheKeys[$key] = true;
+          return $this->writeUserSettingCache($key, $value);
-          $this->settingCache[$key] = $value;
-
-          return $value;
         } catch (Exception $ex) {
           // Fall through below and return the default value.
         }
+      } else {
+        // This is an ad-hoc setting with no controlling object.
+        return $this->writeUserSettingCache($key, $value);
       }
     }
 
@@ -523,10 +523,7 @@ final class PhabricatorUser
       $value = null;
     }
 
-    $this->settingCacheKeys[$key] = true;
+    return $this->writeUserSettingCache($key, $value);
-    $this->settingCache[$key] = $value;
-
-    return $value;
   }
 
 
@@ -557,6 +554,12 @@ final class PhabricatorUser
     return $this;
   }
 
+  private function writeUserSettingCache($key, $value) {
+    $this->settingCacheKeys[$key] = true;
+    $this->settingCache[$key] = $value;
+    return $value;
+  }
+
   public function getTranslation() {
     return $this->getUserSetting(PhabricatorTranslationSetting::SETTINGKEY);
   }