(stable) Promote 2018 Week 10

bin/lock

@@ -0,0 +1 @@
+../scripts/setup/manage_lock.php

resources/sql/autopatches/20140212.dx.1.armageddon.php

@@ -75,7 +75,7 @@ foreach ($rows as $row) {
 
   if ($diff_id || $row['action'] == DifferentialAction::ACTION_UPDATE) {
     $xactions[] = array(
-      'type' => DifferentialTransaction::TYPE_UPDATE,
+      'type' => DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE,
       'old' => null,
       'new' => $diff_id,
     );

resources/sql/autopatches/20180305.lock.01.locklog.sql

@@ -0,0 +1,9 @@
+CREATE TABLE {$NAMESPACE}_daemon.daemon_locklog (
+  id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
+  lockName VARCHAR(64) NOT NULL COLLATE {$COLLATE_TEXT},
+  lockReleased INT UNSIGNED,
+  lockParameters LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT},
+  lockContext LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT},
+  dateCreated INT UNSIGNED NOT NULL,
+  dateModified INT UNSIGNED NOT NULL
+) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT};

resources/sql/autopatches/20180306.opath.01.digest.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_owners.owners_path
+  ADD pathIndex BINARY(12) NOT NULL;

resources/sql/autopatches/20180306.opath.02.digestpopulate.php

@@ -0,0 +1,19 @@
+<?php
+
+$table = new PhabricatorOwnersPath();
+$conn = $table->establishConnection('w');
+
+foreach (new LiskMigrationIterator($table) as $path) {
+  $index = PhabricatorHash::digestForIndex($path->getPath());
+
+  if ($index === $path->getPathIndex()) {
+    continue;
+  }
+
+  queryfx(
+    $conn,
+    'UPDATE %T SET pathIndex = %s WHERE id = %d',
+    $table->getTableName(),
+    $index,
+    $path->getID());
+}

resources/sql/autopatches/20180306.opath.03.purge.php

@@ -0,0 +1,22 @@
+<?php
+
+$table = new PhabricatorOwnersPath();
+$conn = $table->establishConnection('w');
+
+$seen = array();
+foreach (new LiskMigrationIterator($table) as $path) {
+  $package_id = $path->getPackageID();
+  $repository_phid = $path->getRepositoryPHID();
+  $path_index = $path->getPathIndex();
+
+  if (!isset($seen[$package_id][$repository_phid][$path_index])) {
+    $seen[$package_id][$repository_phid][$path_index] = true;
+    continue;
+  }
+
+  queryfx(
+    $conn,
+    'DELETE FROM %T WHERE id = %d',
+    $table->getTableName(),
+    $path->getID());
+}

resources/sql/autopatches/20180306.opath.04.unique.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_owners.owners_path
+  ADD UNIQUE KEY `key_path` (packageID, repositoryPHID, pathIndex);

resources/sql/autopatches/20180306.opath.05.longpath.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_owners.owners_path
+  CHANGE path path LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT};

resources/sql/autopatches/20180306.opath.06.pathdisplay.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_owners.owners_path
+  ADD pathDisplay LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT};

resources/sql/autopatches/20180306.opath.07.copypaths.sql

@@ -0,0 +1,2 @@
+UPDATE {$NAMESPACE}_owners.owners_path
+  SET pathDisplay = path WHERE pathDisplay = '';

resources/sql/autopatches/20180309.owners.01.primaryowner.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_owners.owners_package
+  DROP primaryOwnerPHID;

scripts/setup/manage_lock.php

@@ -0,0 +1,21 @@
+#!/usr/bin/env php
+<?php
+
+$root = dirname(dirname(dirname(__FILE__)));
+require_once $root.'/scripts/init/init-script.php';
+
+$args = new PhutilArgumentParser($argv);
+$args->setTagline(pht('manage locks'));
+$args->setSynopsis(<<<EOSYNOPSIS
+**lock** __command__ [__options__]
+  Manage locks.
+
+EOSYNOPSIS
+  );
+$args->parseStandardArguments();
+
+$workflows = id(new PhutilClassMapQuery())
+  ->setAncestorClass('PhabricatorLockManagementWorkflow')
+  ->execute();
+$workflows[] = new PhutilHelpArgumentWorkflow();
+$args->parseWorkflows($workflows);

src/__phutil_library_map__.php

@@ -598,6 +598,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionTitleTransaction' => 'applications/differential/xaction/DifferentialRevisionTitleTransaction.php',
     'DifferentialRevisionTransactionType' => 'applications/differential/xaction/DifferentialRevisionTransactionType.php',
     'DifferentialRevisionUpdateHistoryView' => 'applications/differential/view/DifferentialRevisionUpdateHistoryView.php',
+    'DifferentialRevisionUpdateTransaction' => 'applications/differential/xaction/DifferentialRevisionUpdateTransaction.php',
     'DifferentialRevisionViewController' => 'applications/differential/controller/DifferentialRevisionViewController.php',
     'DifferentialRevisionVoidTransaction' => 'applications/differential/xaction/DifferentialRevisionVoidTransaction.php',
     'DifferentialRevisionWrongStateTransaction' => 'applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php',
@@ -1882,6 +1883,7 @@ phutil_register_library_map(array(
     'PHUIPropertyGroupView' => 'view/phui/PHUIPropertyGroupView.php',
     'PHUIPropertyListExample' => 'applications/uiexample/examples/PHUIPropertyListExample.php',
     'PHUIPropertyListView' => 'view/phui/PHUIPropertyListView.php',
+    'PHUIRemarkupImageView' => 'infrastructure/markup/view/PHUIRemarkupImageView.php',
     'PHUIRemarkupPreviewPanel' => 'view/phui/PHUIRemarkupPreviewPanel.php',
     'PHUIRemarkupView' => 'infrastructure/markup/view/PHUIRemarkupView.php',
     'PHUISegmentBarSegmentView' => 'view/phui/PHUISegmentBarSegmentView.php',
@@ -2670,6 +2672,8 @@ phutil_register_library_map(array(
     'PhabricatorDaemonController' => 'applications/daemon/controller/PhabricatorDaemonController.php',
     'PhabricatorDaemonDAO' => 'applications/daemon/storage/PhabricatorDaemonDAO.php',
     'PhabricatorDaemonEventListener' => 'applications/daemon/event/PhabricatorDaemonEventListener.php',
+    'PhabricatorDaemonLockLog' => 'applications/daemon/storage/PhabricatorDaemonLockLog.php',
+    'PhabricatorDaemonLockLogGarbageCollector' => 'applications/daemon/garbagecollector/PhabricatorDaemonLockLogGarbageCollector.php',
     'PhabricatorDaemonLog' => 'applications/daemon/storage/PhabricatorDaemonLog.php',
     'PhabricatorDaemonLogEvent' => 'applications/daemon/storage/PhabricatorDaemonLogEvent.php',
     'PhabricatorDaemonLogEventGarbageCollector' => 'applications/daemon/garbagecollector/PhabricatorDaemonLogEventGarbageCollector.php',
@@ -3204,6 +3208,8 @@ phutil_register_library_map(array(
     'PhabricatorLocalTimeTestCase' => 'view/__tests__/PhabricatorLocalTimeTestCase.php',
     'PhabricatorLocaleScopeGuard' => 'infrastructure/internationalization/scope/PhabricatorLocaleScopeGuard.php',
     'PhabricatorLocaleScopeGuardTestCase' => 'infrastructure/internationalization/scope/__tests__/PhabricatorLocaleScopeGuardTestCase.php',
+    'PhabricatorLockLogManagementWorkflow' => 'applications/daemon/management/PhabricatorLockLogManagementWorkflow.php',
+    'PhabricatorLockManagementWorkflow' => 'applications/daemon/management/PhabricatorLockManagementWorkflow.php',
     'PhabricatorLogTriggerAction' => 'infrastructure/daemon/workers/action/PhabricatorLogTriggerAction.php',
     'PhabricatorLogoutController' => 'applications/auth/controller/PhabricatorLogoutController.php',
     'PhabricatorLunarPhasePolicyRule' => 'applications/policy/rule/PhabricatorLunarPhasePolicyRule.php',
@@ -3289,6 +3295,7 @@ phutil_register_library_map(array(
     'PhabricatorMarkupInterface' => 'infrastructure/markup/PhabricatorMarkupInterface.php',
     'PhabricatorMarkupOneOff' => 'infrastructure/markup/PhabricatorMarkupOneOff.php',
     'PhabricatorMarkupPreviewController' => 'infrastructure/markup/PhabricatorMarkupPreviewController.php',
+    'PhabricatorMemeEngine' => 'applications/macro/engine/PhabricatorMemeEngine.php',
     'PhabricatorMemeRemarkupRule' => 'applications/macro/markup/PhabricatorMemeRemarkupRule.php',
     'PhabricatorMentionRemarkupRule' => 'applications/people/markup/PhabricatorMentionRemarkupRule.php',
     'PhabricatorMentionableInterface' => 'applications/transactions/interface/PhabricatorMentionableInterface.php',
@@ -5818,6 +5825,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionTitleTransaction' => 'DifferentialRevisionTransactionType',
     'DifferentialRevisionTransactionType' => 'PhabricatorModularTransactionType',
     'DifferentialRevisionUpdateHistoryView' => 'AphrontView',
+    'DifferentialRevisionUpdateTransaction' => 'DifferentialRevisionTransactionType',
     'DifferentialRevisionViewController' => 'DifferentialController',
     'DifferentialRevisionVoidTransaction' => 'DifferentialRevisionTransactionType',
     'DifferentialRevisionWrongStateTransaction' => 'DifferentialRevisionTransactionType',
@@ -7281,6 +7289,7 @@ phutil_register_library_map(array(
     'PHUIPropertyGroupView' => 'AphrontTagView',
     'PHUIPropertyListExample' => 'PhabricatorUIExample',
     'PHUIPropertyListView' => 'AphrontView',
+    'PHUIRemarkupImageView' => 'AphrontView',
     'PHUIRemarkupPreviewPanel' => 'AphrontTagView',
     'PHUIRemarkupView' => 'AphrontView',
     'PHUISegmentBarSegmentView' => 'AphrontTagView',
@@ -8194,6 +8203,8 @@ phutil_register_library_map(array(
     'PhabricatorDaemonController' => 'PhabricatorController',
     'PhabricatorDaemonDAO' => 'PhabricatorLiskDAO',
     'PhabricatorDaemonEventListener' => 'PhabricatorEventListener',
+    'PhabricatorDaemonLockLog' => 'PhabricatorDaemonDAO',
+    'PhabricatorDaemonLockLogGarbageCollector' => 'PhabricatorGarbageCollector',
     'PhabricatorDaemonLog' => array(
       'PhabricatorDaemonDAO',
       'PhabricatorPolicyInterface',
@@ -8794,6 +8805,8 @@ phutil_register_library_map(array(
     'PhabricatorLocalTimeTestCase' => 'PhabricatorTestCase',
     'PhabricatorLocaleScopeGuard' => 'Phobject',
     'PhabricatorLocaleScopeGuardTestCase' => 'PhabricatorTestCase',
+    'PhabricatorLockLogManagementWorkflow' => 'PhabricatorLockManagementWorkflow',
+    'PhabricatorLockManagementWorkflow' => 'PhabricatorManagementWorkflow',
     'PhabricatorLogTriggerAction' => 'PhabricatorTriggerAction',
     'PhabricatorLogoutController' => 'PhabricatorAuthController',
     'PhabricatorLunarPhasePolicyRule' => 'PhabricatorPolicyRule',
@@ -8881,6 +8894,7 @@ phutil_register_library_map(array(
       'PhabricatorMarkupInterface',
     ),
     'PhabricatorMarkupPreviewController' => 'PhabricatorController',
+    'PhabricatorMemeEngine' => 'Phobject',
     'PhabricatorMemeRemarkupRule' => 'PhutilRemarkupRule',
     'PhabricatorMentionRemarkupRule' => 'PhutilRemarkupRule',
     'PhabricatorMercurialGraphStream' => 'PhabricatorRepositoryGraphStream',

src/applications/daemon/garbagecollector/PhabricatorDaemonLockLogGarbageCollector.php

@@ -0,0 +1,29 @@
+<?php
+
+final class PhabricatorDaemonLockLogGarbageCollector
+  extends PhabricatorGarbageCollector {
+
+  const COLLECTORCONST = 'daemon.lock-log';
+
+  public function getCollectorName() {
+    return pht('Lock Logs');
+  }
+
+  public function getDefaultRetentionPolicy() {
+    return 0;
+  }
+
+  protected function collectGarbage() {
+    $table = new PhabricatorDaemonLockLog();
+    $conn = $table->establishConnection('w');
+
+    queryfx(
+      $conn,
+      'DELETE FROM %T WHERE dateCreated < %d LIMIT 100',
+      $table->getTableName(),
+      $this->getGarbageEpoch());
+
+    return ($conn->getAffectedRows() == 100);
+  }
+
+}

src/applications/daemon/management/PhabricatorLockLogManagementWorkflow.php

@@ -0,0 +1,222 @@
+<?php
+
+final class PhabricatorLockLogManagementWorkflow
+  extends PhabricatorLockManagementWorkflow {
+
+  protected function didConstruct() {
+    $this
+      ->setName('log')
+      ->setSynopsis(pht('Enable, disable, or show the lock log.'))
+      ->setArguments(
+        array(
+          array(
+            'name' => 'enable',
+            'help' => pht('Enable the lock log.'),
+          ),
+          array(
+            'name' => 'disable',
+            'help' => pht('Disable the lock log.'),
+          ),
+          array(
+            'name' => 'name',
+            'param' => 'name',
+            'help' => pht('Review logs for a specific lock.'),
+          ),
+        ));
+  }
+
+  public function execute(PhutilArgumentParser $args) {
+    $is_enable = $args->getArg('enable');
+    $is_disable = $args->getArg('disable');
+
+    if ($is_enable && $is_disable) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'You can not both "--enable" and "--disable" the lock log.'));
+    }
+
+    $with_name = $args->getArg('name');
+
+    if ($is_enable || $is_disable) {
+      if (strlen($with_name)) {
+        throw new PhutilArgumentUsageException(
+          pht(
+            'You can not both "--enable" or "--disable" with search '.
+            'parameters like "--name".'));
+      }
+
+      $gc = new PhabricatorDaemonLockLogGarbageCollector();
+      $is_enabled = (bool)$gc->getRetentionPolicy();
+
+      $config_key = 'phd.garbage-collection';
+      $const = $gc->getCollectorConstant();
+      $value = PhabricatorEnv::getEnvConfig($config_key);
+
+      if ($is_disable) {
+        if (!$is_enabled) {
+          echo tsprintf(
+            "%s\n",
+            pht('Lock log is already disabled.'));
+          return 0;
+        }
+        echo tsprintf(
+          "%s\n",
+          pht('Disabling the lock log.'));
+
+        unset($value[$const]);
+      } else {
+        if ($is_enabled) {
+          echo tsprintf(
+            "%s\n",
+            pht('Lock log is already enabled.'));
+          return 0;
+        }
+        echo tsprintf(
+          "%s\n",
+          pht('Enabling the lock log.'));
+
+        $value[$const] = phutil_units('24 hours in seconds');
+      }
+
+      id(new PhabricatorConfigLocalSource())
+        ->setKeys(
+          array(
+            $config_key => $value,
+          ));
+
+      echo tsprintf(
+        "%s\n",
+        pht('Done.'));
+
+      echo tsprintf(
+        "%s\n",
+        pht('Restart daemons to apply changes.'));
+
+      return 0;
+    }
+
+    $table = new PhabricatorDaemonLockLog();
+    $conn = $table->establishConnection('r');
+
+    $parts = array();
+    if (strlen($with_name)) {
+      $parts[] = qsprintf(
+        $conn,
+        'lockName = %s',
+        $with_name);
+    }
+
+    if (!$parts) {
+      $constraint = '1 = 1';
+    } else {
+      $constraint = '('.implode(') AND (', $parts).')';
+    }
+
+    $logs = $table->loadAllWhere(
+      '%Q ORDER BY id DESC LIMIT 100',
+      $constraint);
+    $logs = array_reverse($logs);
+
+    if (!$logs) {
+      echo tsprintf(
+        "%s\n",
+        pht('No matching lock logs.'));
+      return 0;
+    }
+
+    $table = id(new PhutilConsoleTable())
+      ->setBorders(true)
+      ->addColumn(
+        'id',
+        array(
+          'title' => pht('Lock'),
+        ))
+      ->addColumn(
+        'name',
+        array(
+          'title' => pht('Name'),
+        ))
+      ->addColumn(
+        'acquired',
+        array(
+          'title' => pht('Acquired'),
+        ))
+      ->addColumn(
+        'released',
+        array(
+          'title' => pht('Released'),
+        ))
+      ->addColumn(
+        'held',
+        array(
+          'title' => pht('Held'),
+        ))
+      ->addColumn(
+        'parameters',
+        array(
+          'title' => pht('Parameters'),
+        ))
+      ->addColumn(
+        'context',
+        array(
+          'title' => pht('Context'),
+        ));
+
+    $viewer = $this->getViewer();
+
+    foreach ($logs as $log) {
+      $created = $log->getDateCreated();
+      $released = $log->getLockReleased();
+
+      if ($released) {
+        $held = '+'.($released - $created);
+      } else {
+        $held = null;
+      }
+
+      $created = phabricator_datetime($created, $viewer);
+      $released = phabricator_datetime($released, $viewer);
+
+      $parameters = $log->getLockParameters();
+      $context = $log->getLockContext();
+
+      $table->addRow(
+        array(
+          'id' => $log->getID(),
+          'name' => $log->getLockName(),
+          'acquired' => $created,
+          'released' => $released,
+          'held' => $held,
+          'parameters' => $this->flattenParameters($parameters),
+          'context' => $this->flattenParameters($context),
+        ));
+    }
+
+    $table->draw();
+
+    return 0;
+  }
+
+  private function flattenParameters(array $params, $keys = true) {
+    $flat = array();
+    foreach ($params as $key => $value) {
+      if (is_array($value)) {
+        $value = $this->flattenParameters($value, false);
+      }
+      if ($keys) {
+        $flat[] = "{$key}={$value}";
+      } else {
+        $flat[] = "{$value}";
+      }
+    }
+
+    if ($keys) {
+      $flat = implode(', ', $flat);
+    } else {
+      $flat = implode(' ', $flat);
+    }
+
+    return $flat;
+  }
+
+}

src/applications/daemon/management/PhabricatorLockManagementWorkflow.php

@@ -0,0 +1,4 @@
+<?php
+
+abstract class PhabricatorLockManagementWorkflow
+  extends PhabricatorManagementWorkflow {}

src/applications/daemon/storage/PhabricatorDaemonLockLog.php

@@ -0,0 +1,32 @@
+<?php
+
+final class PhabricatorDaemonLockLog
+  extends PhabricatorDaemonDAO {
+
+  protected $lockName;
+  protected $lockReleased;
+  protected $lockParameters = array();
+  protected $lockContext = array();
+
+  protected function getConfiguration() {
+    return array(
+      self::CONFIG_SERIALIZATION => array(
+        'lockParameters' => self::SERIALIZATION_JSON,
+        'lockContext' => self::SERIALIZATION_JSON,
+      ),
+      self::CONFIG_COLUMN_SCHEMA => array(
+        'lockName' => 'text64',
+        'lockReleased' => 'epoch?',
+      ),
+      self::CONFIG_KEY_SCHEMA => array(
+        'key_lock' => array(
+          'columns' => array('lockName'),
+        ),
+        'key_created' => array(
+          'columns' => array('dateCreated'),
+        ),
+      ),
+    ) + parent::getConfiguration();
+  }
+
+}

src/applications/differential/conduit/DifferentialConduitAPIMethod.php

@@ -58,7 +58,7 @@ abstract class DifferentialConduitAPIMethod extends ConduitAPIMethod {
 
     $xactions = array();
     $xactions[] = array(
-      'type' => DifferentialRevisionEditEngine::KEY_UPDATE,
+      'type' => DifferentialRevisionUpdateTransaction::EDITKEY,
       'value' => $diff->getPHID(),
     );
 

src/applications/differential/constants/DifferentialRevisionStatus.php

@@ -172,10 +172,10 @@ final class DifferentialRevisionStatus extends Phobject {
         'name' => pht('Draft'),
         // For legacy clients, treat this as though it is "Needs Review".
         'legacy' => 0,
-        'icon' => 'fa-file-text-o',
+        'icon' => 'fa-spinner',
         'closed' => false,
-        'color.icon' => 'grey',
+        'color.icon' => 'pink',
-        'color.tag' => 'grey',
+        'color.tag' => 'pink',
         'color.ansi' => null,
       ),
     );

src/applications/differential/controller/DifferentialChangesetViewController.php

@@ -390,9 +390,19 @@ final class DifferentialChangesetViewController extends DifferentialController {
       return array();
     }
 
+    $change_type = $changeset->getChangeType();
+    if (DifferentialChangeType::isDeleteChangeType($change_type)) {
+      // If this is a lint message on a deleted file, show it on the left
+      // side of the UI because there are no source code lines on the right
+      // side of the UI so inlines don't have anywhere to render. See PHI416.
+      $is_new = 0;
+    } else {
+      $is_new = 1;
+    }
+
     $template = id(new DifferentialInlineComment())
       ->setChangesetID($changeset->getID())
-        ->setIsNewFile(1)
+      ->setIsNewFile($is_new)
       ->setLineLength(0);
 
     $inlines = array();

src/applications/differential/customfield/DifferentialDraftField.php

@@ -37,9 +37,14 @@ final class DifferentialDraftField
     }
 
     // If the author has held this revision as a draft explicitly, don't
-    // show any misleading messages about it autosubmitting later.
+    // show any misleading messages about it autosubmitting later. We do show
+    // reminder text.
     if ($revision->getHoldAsDraft()) {
-      return array();
+      return array(
+        pht(
+          'This is a draft revision that has not yet been submitted for '.
+          'review.'),
+      );
     }
 
     $warnings = array();
@@ -93,4 +98,19 @@ final class DifferentialDraftField
     return $warnings;
   }
 
+  public function getWarningsForDetailView() {
+    $revision = $this->getObject();
+
+    if (!$revision->isDraft()) {
+      return array();
+    }
+
+    return array(
+      pht(
+        'This revision is currently a draft. You can leave comments, but '.
+        'no one will be notified until the revision is submitted for '.
+        'review.'),
+    );
+  }
+
 }

src/applications/differential/editor/DifferentialRevisionEditEngine.php

@@ -7,8 +7,6 @@ final class DifferentialRevisionEditEngine
 
   const ENGINECONST = 'differential.revision';
 
-  const KEY_UPDATE = 'update';
-
   const ACTIONGROUP_REVIEW = 'review';
   const ACTIONGROUP_REVISION = 'revision';
 
@@ -73,6 +71,14 @@ final class DifferentialRevisionEditEngine
     return pht('Revision');
   }
 
+  protected function getCommentViewButtonText($object) {
+    if ($object->isDraft()) {
+      return pht('Submit Quietly');
+    }
+
+    return parent::getCommentViewButtonText($object);
+  }
+
   protected function getObjectViewURI($object) {
     return $object->getURI();
   }
@@ -123,12 +129,13 @@ final class DifferentialRevisionEditEngine
     $fields = array();
 
     $fields[] = id(new PhabricatorHandlesEditField())
-      ->setKey(self::KEY_UPDATE)
+      ->setKey(DifferentialRevisionUpdateTransaction::EDITKEY)
       ->setLabel(pht('Update Diff'))
       ->setDescription(pht('New diff to create or update the revision with.'))
       ->setConduitDescription(pht('Create or update a revision with a diff.'))
       ->setConduitTypeDescription(pht('PHID of the diff.'))
-      ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
+      ->setTransactionType(
+        DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE)
       ->setHandleParameterType(new AphrontPHIDListHTTPParameterType())
       ->setSingleValue($diff_phid)
       ->setIsConduitOnly(!$diff)

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -33,7 +33,7 @@ final class DifferentialTransactionEditor
   }
 
   public function getDiffUpdateTransaction(array $xactions) {
-    $type_update = DifferentialTransaction::TYPE_UPDATE;
+    $type_update = DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE;
 
     foreach ($xactions as $xaction) {
       if ($xaction->getTransactionType() == $type_update) {
@@ -76,7 +76,6 @@ final class DifferentialTransactionEditor
     $types[] = PhabricatorTransactions::TYPE_INLINESTATE;
 
     $types[] = DifferentialTransaction::TYPE_INLINE;
-    $types[] = DifferentialTransaction::TYPE_UPDATE;
 
     return $types;
   }
@@ -88,12 +87,6 @@ final class DifferentialTransactionEditor
     switch ($xaction->getTransactionType()) {
       case DifferentialTransaction::TYPE_INLINE:
         return null;
-      case DifferentialTransaction::TYPE_UPDATE:
-        if ($this->getIsNewObject()) {
-          return null;
-        } else {
-          return $object->getActiveDiff()->getPHID();
-        }
     }
 
     return parent::getCustomTransactionOldValue($object, $xaction);
@@ -104,8 +97,6 @@ final class DifferentialTransactionEditor
     PhabricatorApplicationTransaction $xaction) {
 
     switch ($xaction->getTransactionType()) {
-      case DifferentialTransaction::TYPE_UPDATE:
-        return $xaction->getNewValue();
       case DifferentialTransaction::TYPE_INLINE:
         return null;
     }
@@ -120,29 +111,6 @@ final class DifferentialTransactionEditor
     switch ($xaction->getTransactionType()) {
       case DifferentialTransaction::TYPE_INLINE:
         return;
-      case DifferentialTransaction::TYPE_UPDATE:
-        if (!$this->getIsCloseByCommit()) {
-          if ($object->isNeedsRevision() ||
-              $object->isChangePlanned() ||
-              $object->isAbandoned()) {
-            $object->setModernRevisionStatus(
-              DifferentialRevisionStatus::NEEDS_REVIEW);
-          }
-        }
-
-        $diff = $this->requireDiff($xaction->getNewValue());
-
-        $this->updateRevisionLineCounts($object, $diff);
-
-        if ($this->repositoryPHIDOverride !== false) {
-          $object->setRepositoryPHID($this->repositoryPHIDOverride);
-        } else {
-          $object->setRepositoryPHID($diff->getRepositoryPHID());
-        }
-
-        $object->attachActiveDiff($diff);
-        $object->setActiveDiffPHID($diff->getPHID());
-        return;
     }
 
     return parent::applyCustomInternalTransaction($object, $xaction);
@@ -196,7 +164,7 @@ final class DifferentialTransactionEditor
       // commit.
     } else {
       switch ($xaction->getTransactionType()) {
-        case DifferentialTransaction::TYPE_UPDATE:
+        case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
           $downgrade_rejects = true;
           if (!$is_sticky_accept) {
             // If "sticky accept" is disabled, also downgrade the accepts.
@@ -243,7 +211,7 @@ final class DifferentialTransactionEditor
 
     $is_commandeer = false;
     switch ($xaction->getTransactionType()) {
-      case DifferentialTransaction::TYPE_UPDATE:
+      case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
         if ($this->getIsCloseByCommit()) {
           // Don't bother with any of this if this update is a side effect of
           // commit detection.
@@ -293,7 +261,7 @@ final class DifferentialTransactionEditor
     if (!$this->didExpandInlineState) {
       switch ($xaction->getTransactionType()) {
         case PhabricatorTransactions::TYPE_COMMENT:
-        case DifferentialTransaction::TYPE_UPDATE:
+        case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
         case DifferentialTransaction::TYPE_INLINE:
           $this->didExpandInlineState = true;
 
@@ -343,45 +311,6 @@ final class DifferentialTransactionEditor
         if ($reply && !$reply->getHasReplies()) {
           $reply->setHasReplies(1)->save();
         }
-        return;
-      case DifferentialTransaction::TYPE_UPDATE:
-        // Now that we're inside the transaction, do a final check.
-        $diff = $this->requireDiff($xaction->getNewValue());
-
-        // TODO: It would be slightly cleaner to just revalidate this
-        // transaction somehow using the same validation code, but that's
-        // not easy to do at the moment.
-
-        $revision_id = $diff->getRevisionID();
-        if ($revision_id && ($revision_id != $object->getID())) {
-          throw new Exception(
-            pht(
-              'Diff is already attached to another revision. You lost '.
-              'a race?'));
-        }
-
-        // TODO: This can race with diff updates, particularly those from
-        // Harbormaster. See discussion in T8650.
-        $diff->setRevisionID($object->getID());
-        $diff->save();
-
-        // If there are any outstanding buildables for this diff, tell
-        // Harbormaster that their containers need to be updated. This is
-        // common, because `arc` creates buildables so it can upload lint
-        // and unit results.
-
-        $buildables = id(new HarbormasterBuildableQuery())
-          ->setViewer(PhabricatorUser::getOmnipotentUser())
-          ->withManualBuildables(false)
-          ->withBuildablePHIDs(array($diff->getPHID()))
-          ->execute();
-        foreach ($buildables as $buildable) {
-          $buildable->sendMessage(
-            $this->getActor(),
-            HarbormasterMessageType::BUILDABLE_CONTAINER,
-            true);
-        }
-
         return;
     }
 
@@ -437,7 +366,7 @@ final class DifferentialTransactionEditor
 
     foreach ($xactions as $xaction) {
       switch ($xaction->getTransactionType()) {
-        case DifferentialTransaction::TYPE_UPDATE:
+        case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
           $diff = $this->requireDiff($xaction->getNewValue(), true);
 
           // Update these denormalized index tables when we attach a new
@@ -554,44 +483,6 @@ final class DifferentialTransactionEditor
     return $xactions;
   }
 
-
-  protected function validateTransaction(
-    PhabricatorLiskDAO $object,
-    $type,
-    array $xactions) {
-
-    $errors = parent::validateTransaction($object, $type, $xactions);
-
-    $config_self_accept_key = 'differential.allow-self-accept';
-    $allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key);
-
-    foreach ($xactions as $xaction) {
-      switch ($type) {
-        case DifferentialTransaction::TYPE_UPDATE:
-          $diff = $this->loadDiff($xaction->getNewValue());
-          if (!$diff) {
-            $errors[] = new PhabricatorApplicationTransactionValidationError(
-              $type,
-              pht('Invalid'),
-              pht('The specified diff does not exist.'),
-              $xaction);
-          } else if (($diff->getRevisionID()) &&
-            ($diff->getRevisionID() != $object->getID())) {
-            $errors[] = new PhabricatorApplicationTransactionValidationError(
-              $type,
-              pht('Invalid'),
-              pht(
-                'You can not update this revision to the specified diff, '.
-                'because the diff is already attached to another revision.'),
-              $xaction);
-          }
-          break;
-      }
-    }
-
-    return $errors;
-  }
-
   protected function sortTransactions(array $xactions) {
     $xactions = parent::sortTransactions($xactions);
 
@@ -674,7 +565,7 @@ final class DifferentialTransactionEditor
       $action = parent::getMailAction($object, $xactions);
 
       $strongest = $this->getStrongestAction($object, $xactions);
-      $type_update = DifferentialTransaction::TYPE_UPDATE;
+      $type_update = DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE;
       if ($strongest->getTransactionType() == $type_update) {
         $show_lines = true;
       }
@@ -772,7 +663,7 @@ final class DifferentialTransactionEditor
     $update_xaction = null;
     foreach ($xactions as $xaction) {
       switch ($xaction->getTransactionType()) {
-        case DifferentialTransaction::TYPE_UPDATE:
+        case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
           $update_xaction = $xaction;
           break;
       }
@@ -1053,7 +944,7 @@ final class DifferentialTransactionEditor
     return $query->executeOne();
   }
 
-  private function requireDiff($phid, $need_changesets = false) {
+  public function requireDiff($phid, $need_changesets = false) {
     $diff = $this->loadDiff($phid, $need_changesets);
     if (!$diff) {
       throw new Exception(pht('Diff "%s" does not exist!', $phid));
@@ -1091,11 +982,28 @@ final class DifferentialTransactionEditor
       return array();
     }
 
-    // Remove packages that the revision author is an owner of. If you own
+    // Identify the packages with "Non-Owner Author" review rules and remove
-    // code, you don't need another owner to review it.
+    // them if the author has authority over the package.
+
+    $autoreview_map = PhabricatorOwnersPackage::getAutoreviewOptionsMap();
+    $need_authority = array();
+    foreach ($packages as $package) {
+      $autoreview_setting = $package->getAutoReview();
+
+      $spec = idx($autoreview_map, $autoreview_setting);
+      if (!$spec) {
+        continue;
+      }
+
+      if (idx($spec, 'authority')) {
+        $need_authority[$package->getPHID()] = $package->getPHID();
+      }
+    }
+
+    if ($need_authority) {
       $authority = id(new PhabricatorOwnersPackageQuery())
         ->setViewer(PhabricatorUser::getOmnipotentUser())
-      ->withPHIDs(mpull($packages, 'getPHID'))
+        ->withPHIDs($need_authority)
         ->withAuthorityPHIDs(array($object->getAuthorPHID()))
         ->execute();
       $authority = mpull($authority, null, 'getPHID');
@@ -1111,6 +1019,7 @@ final class DifferentialTransactionEditor
       if (!$packages) {
         return array();
       }
+    }
 
     $auto_subscribe = array();
     $auto_review = array();
@@ -1118,15 +1027,18 @@ final class DifferentialTransactionEditor
 
     foreach ($packages as $package) {
       switch ($package->getAutoReview()) {
-        case PhabricatorOwnersPackage::AUTOREVIEW_SUBSCRIBE:
-          $auto_subscribe[] = $package;
-          break;
         case PhabricatorOwnersPackage::AUTOREVIEW_REVIEW:
+        case PhabricatorOwnersPackage::AUTOREVIEW_REVIEW_ALWAYS:
           $auto_review[] = $package;
           break;
         case PhabricatorOwnersPackage::AUTOREVIEW_BLOCK:
+        case PhabricatorOwnersPackage::AUTOREVIEW_BLOCK_ALWAYS:
           $auto_block[] = $package;
           break;
+        case PhabricatorOwnersPackage::AUTOREVIEW_SUBSCRIBE:
+        case PhabricatorOwnersPackage::AUTOREVIEW_SUBSCRIBE_ALWAYS:
+          $auto_subscribe[] = $package;
+          break;
         case PhabricatorOwnersPackage::AUTOREVIEW_NONE:
         default:
           break;
@@ -1274,7 +1186,7 @@ final class DifferentialTransactionEditor
     $has_update = false;
     $has_commit = false;
 
-    $type_update = DifferentialTransaction::TYPE_UPDATE;
+    $type_update = DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE;
     foreach ($xactions as $xaction) {
       if ($xaction->getTransactionType() != $type_update) {
         continue;
@@ -1721,27 +1633,6 @@ final class DifferentialTransactionEditor
     return true;
   }
 
-  private function updateRevisionLineCounts(
-    DifferentialRevision $revision,
-    DifferentialDiff $diff) {
-
-    $revision->setLineCount($diff->getLineCount());
-
-    $conn = $revision->establishConnection('r');
-
-    $row = queryfx_one(
-      $conn,
-      'SELECT SUM(addLines) A, SUM(delLines) D FROM %T
-        WHERE diffID = %d',
-      id(new DifferentialChangeset())->getTableName(),
-      $diff->getID());
-
-    if ($row) {
-      $revision->setAddedLineCount((int)$row['A']);
-      $revision->setRemovedLineCount((int)$row['D']);
-    }
-  }
-
   private function requireReviewers(DifferentialRevision $revision) {
     if ($revision->hasAttachedReviewers()) {
       return;

src/applications/differential/engine/DifferentialDiffExtractionEngine.php

@@ -278,11 +278,14 @@ final class DifferentialDiffExtractionEngine extends Phobject {
         ->setNewValue($revision->getModernRevisionStatus());
     }
 
+    $type_update = DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE;
+
     $xactions[] = id(new DifferentialTransaction())
-      ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
+      ->setTransactionType($type_update)
       ->setIgnoreOnNoEffect(true)
       ->setNewValue($new_diff->getPHID())
-      ->setMetadataValue('isCommitUpdate', true);
+      ->setMetadataValue('isCommitUpdate', true)
+      ->setMetadataValue('commitPHIDs', array($commit->getPHID()));
 
     foreach ($more_xactions as $more_xaction) {
       $xactions[] = $more_xaction;

src/applications/differential/lipsum/PhabricatorDifferentialRevisionTestDataGenerator.php

@@ -22,14 +22,14 @@ final class PhabricatorDifferentialRevisionTestDataGenerator
     $revision->setTestPlan($this->generateDescription());
 
     $diff = $this->generateDiff($author);
+    $type_update = DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE;
 
     $xactions = array();
 
     $xactions[] = id(new DifferentialTransaction())
-      ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
+      ->setTransactionType($type_update)
       ->setNewValue($diff->getPHID());
 
-
     id(new DifferentialTransactionEditor())
       ->setActor($author)
       ->setContentSource($this->getLipsumContentSource())

src/applications/differential/storage/DifferentialTransaction.php

@@ -6,7 +6,6 @@ final class DifferentialTransaction
   private $isCommandeerSideEffect;
 
   const TYPE_INLINE  = 'differential:inline';
-  const TYPE_UPDATE  = 'differential:update';
   const TYPE_ACTION  = 'differential:action';
 
   const MAILTAG_REVIEWERS      = 'differential-reviewers';
@@ -75,18 +74,6 @@ final class DifferentialTransaction
     $new = $this->getNewValue();
 
     switch ($this->getTransactionType()) {
-      case self::TYPE_UPDATE:
-        // Older versions of this transaction have an ID for the new value,
-        // and/or do not record the old value. Only hide the transaction if
-        // the new value is a PHID, indicating that this is a newer style
-        // transaction.
-        if ($old === null) {
-          if (phid_get_type($new) == DifferentialDiffPHIDType::TYPECONST) {
-            return true;
-          }
-        }
-        break;
-
       case DifferentialRevisionRequestReviewTransaction::TRANSACTIONTYPE:
         // Don't hide the initial "X requested review: ..." transaction from
         // mail or feed even when it occurs during creation. We need this
@@ -139,11 +126,6 @@ final class DifferentialTransaction
           }
         }
         break;
-      case self::TYPE_UPDATE:
-        if ($new) {
-          $phids[] = $new;
-        }
-        break;
     }
 
     return $phids;
@@ -153,8 +135,6 @@ final class DifferentialTransaction
     switch ($this->getTransactionType()) {
       case self::TYPE_ACTION:
         return 3;
-      case self::TYPE_UPDATE:
-        return 2;
     }
 
     return parent::getActionStrength();
@@ -165,13 +145,6 @@ final class DifferentialTransaction
     switch ($this->getTransactionType()) {
       case self::TYPE_INLINE:
         return pht('Commented On');
-      case self::TYPE_UPDATE:
-        $old = $this->getOldValue();
-        if ($old === null) {
-          return pht('Request');
-        } else {
-          return pht('Updated');
-        }
       case self::TYPE_ACTION:
         $map = array(
           DifferentialAction::ACTION_ACCEPT => pht('Accepted'),
@@ -209,7 +182,7 @@ final class DifferentialTransaction
             break;
         }
         break;
-      case self::TYPE_UPDATE:
+      case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
         $old = $this->getOldValue();
         if ($old === null) {
           $tags[] = self::MAILTAG_REVIEW_REQUEST;
@@ -248,28 +221,6 @@ final class DifferentialTransaction
         return pht(
           '%s added inline comments.',
           $author_handle);
-      case self::TYPE_UPDATE:
-        if ($this->getMetadataValue('isCommitUpdate')) {
-          return pht(
-            'This revision was automatically updated to reflect the '.
-            'committed changes.');
-        } else if ($new) {
-          // TODO: Migrate to PHIDs and use handles here?
-          if (phid_get_type($new) == DifferentialDiffPHIDType::TYPECONST) {
-            return pht(
-              '%s updated this revision to %s.',
-              $author_handle,
-              $this->renderHandleLink($new));
-          } else {
-            return pht(
-              '%s updated this revision.',
-              $author_handle);
-          }
-        } else {
-          return pht(
-            '%s updated this revision.',
-            $author_handle);
-        }
       case self::TYPE_ACTION:
         switch ($new) {
           case DifferentialAction::ACTION_CLOSE:
@@ -347,11 +298,6 @@ final class DifferentialTransaction
           '%s added inline comments to %s.',
           $author_link,
           $object_link);
-      case self::TYPE_UPDATE:
-        return pht(
-          '%s updated the diff for %s.',
-          $author_link,
-          $object_link);
       case self::TYPE_ACTION:
         switch ($new) {
           case DifferentialAction::ACTION_ACCEPT:
@@ -462,8 +408,6 @@ final class DifferentialTransaction
     switch ($this->getTransactionType()) {
       case self::TYPE_INLINE:
         return 'fa-comment';
-      case self::TYPE_UPDATE:
-        return 'fa-refresh';
       case self::TYPE_ACTION:
         switch ($this->getNewValue()) {
           case DifferentialAction::ACTION_CLOSE:
@@ -526,8 +470,6 @@ final class DifferentialTransaction
 
   public function getColor() {
     switch ($this->getTransactionType()) {
-      case self::TYPE_UPDATE:
-        return PhabricatorTransactions::COLOR_SKY;
       case self::TYPE_ACTION:
         switch ($this->getNewValue()) {
           case DifferentialAction::ACTION_CLOSE:

src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php

@@ -136,4 +136,23 @@ final class DifferentialRevisionCloseTransaction
       $this->renderObject());
   }
 
+  public function getTransactionTypeForConduit($xaction) {
+    return 'close';
+  }
+
+  public function getFieldValuesForConduit($object, $data) {
+    $commit_phid = $object->getMetadataValue('commitPHID');
+
+    if ($commit_phid) {
+      $commit_phids = array($commit_phid);
+    } else {
+      $commit_phids = array();
+    }
+
+    return array(
+      'commitPHIDs' => $commit_phids,
+    );
+  }
+
+
 }

src/applications/differential/xaction/DifferentialRevisionUpdateTransaction.php

@@ -0,0 +1,199 @@
+<?php
+
+final class DifferentialRevisionUpdateTransaction
+  extends DifferentialRevisionTransactionType {
+
+  const TRANSACTIONTYPE = 'differential:update';
+  const EDITKEY = 'update';
+
+  public function generateOldValue($object) {
+    return $object->getActiveDiffPHID();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $should_review = $this->shouldRequestReviewAfterUpdate($object);
+    if ($should_review) {
+      $object->setModernRevisionStatus(
+        DifferentialRevisionStatus::NEEDS_REVIEW);
+    }
+
+    $editor = $this->getEditor();
+    $diff = $editor->requireDiff($value);
+
+    $this->updateRevisionLineCounts($object, $diff);
+
+    $object->setRepositoryPHID($diff->getRepositoryPHID());
+    $object->setActiveDiffPHID($diff->getPHID());
+    $object->attachActiveDiff($diff);
+  }
+
+  private function shouldRequestReviewAfterUpdate($object) {
+    if ($this->isCommitUpdate()) {
+      return false;
+    }
+
+    $should_update =
+      $object->isNeedsRevision() ||
+      $object->isChangePlanned() ||
+      $object->isAbandoned();
+    if ($should_update) {
+      return true;
+    }
+
+    return false;
+  }
+
+  public function applyExternalEffects($object, $value) {
+    $editor = $this->getEditor();
+    $diff = $editor->requireDiff($value);
+
+    // TODO: This can race with diff updates, particularly those from
+    // Harbormaster. See discussion in T8650.
+    $diff->setRevisionID($object->getID());
+    $diff->save();
+
+    // If there are any outstanding buildables for this diff, tell
+    // Harbormaster that their containers need to be updated. This is
+    // common, because `arc` creates buildables so it can upload lint
+    // and unit results.
+
+    $buildables = id(new HarbormasterBuildableQuery())
+      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->withManualBuildables(false)
+      ->withBuildablePHIDs(array($diff->getPHID()))
+      ->execute();
+    foreach ($buildables as $buildable) {
+      $buildable->sendMessage(
+        $this->getActor(),
+        HarbormasterMessageType::BUILDABLE_CONTAINER,
+        true);
+    }
+  }
+
+  public function getColor() {
+    return 'sky';
+  }
+
+  public function getIcon() {
+    return 'fa-refresh';
+  }
+
+  public function getActionName() {
+    if ($this->isCreateTransaction()) {
+      return pht('Request');
+    } else {
+      return pht('Updated');
+    }
+  }
+
+  public function getActionStrength() {
+    return 2;
+  }
+
+  public function getTitle() {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    if ($this->isCommitUpdate()) {
+      return pht(
+        'This revision was automatically updated to reflect the '.
+        'committed changes.');
+    }
+
+    // NOTE: Very, very old update transactions did not have a new value or
+    // did not use a diff PHID as a new value. This was changed years ago,
+    // but wasn't migrated. We might consider migrating if this causes issues.
+
+    return pht(
+      '%s updated this revision to %s.',
+      $this->renderAuthor(),
+      $this->renderNewHandle());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s updated the diff for %s.',
+      $this->renderAuthor(),
+      $this->renderObject());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    $diff_phid = null;
+    foreach ($xactions as $xaction) {
+      $diff_phid = $xaction->getNewValue();
+
+      $diff = id(new DifferentialDiffQuery())
+        ->withPHIDs(array($diff_phid))
+        ->setViewer($this->getActor())
+        ->executeOne();
+      if (!$diff) {
+        $errors[] = $this->newInvalidError(
+          pht(
+            'Specified diff ("%s") does not exist.',
+            $diff_phid),
+          $xaction);
+        continue;
+      }
+
+      if ($diff->getRevisionID()) {
+        $errors[] = $this->newInvalidError(
+          pht(
+            'You can not update this revision with the specified diff ("%s") '.
+            'because the diff is already attached to another revision.',
+            $diff_phid),
+          $xaction);
+        continue;
+      }
+    }
+
+    if (!$diff_phid && !$object->getActiveDiffPHID()) {
+      $errors[] = $this->newInvalidError(
+        pht(
+          'You must specify an initial diff when creating a revision.'));
+    }
+
+    return $errors;
+  }
+
+  public function isCommitUpdate() {
+    return (bool)$this->getMetadataValue('isCommitUpdate');
+  }
+
+  private function updateRevisionLineCounts(
+    DifferentialRevision $revision,
+    DifferentialDiff $diff) {
+
+    $revision->setLineCount($diff->getLineCount());
+
+    $conn = $revision->establishConnection('r');
+
+    $row = queryfx_one(
+      $conn,
+      'SELECT SUM(addLines) A, SUM(delLines) D FROM %T
+        WHERE diffID = %d',
+      id(new DifferentialChangeset())->getTableName(),
+      $diff->getID());
+
+    if ($row) {
+      $revision->setAddedLineCount((int)$row['A']);
+      $revision->setRemovedLineCount((int)$row['D']);
+    }
+  }
+
+  public function getTransactionTypeForConduit($xaction) {
+    return 'update';
+  }
+
+  public function getFieldValuesForConduit($object, $data) {
+    $commit_phids = $object->getMetadataValue('commitPHIDs', array());
+
+    return array(
+      'old' => $object->getOldValue(),
+      'new' => $object->getNewValue(),
+      'commitPHIDs' => $commit_phids,
+    );
+  }
+
+}

src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php

@@ -35,7 +35,8 @@ final class DifferentialRevisionWrongStateTransaction
       $this->renderValue($status->getDisplayName()));
   }
 
-  public function getTitleForFeed() {
+  public function shouldHideForFeed() {
-    return null;
+    return true;
   }
+
 }

src/applications/diffusion/controller/DiffusionController.php

@@ -69,6 +69,10 @@ abstract class DiffusionController extends PhabricatorController {
     // repository has a different canonical path like "/diffusion/XYZ/...",
     // redirect them to the canonical path.
 
+    // Skip this redirect if the request is an AJAX request, like the requests
+    // that Owners makes to complete and validate paths.
+
+    if (!$request->isAjax()) {
       $request_path = $request->getPath();
       $repository = $drequest->getRepository();
 
@@ -78,6 +82,7 @@ abstract class DiffusionController extends PhabricatorController {
           return id(new AphrontRedirectResponse())->setURI($canonical_path);
         }
       }
+    }
 
     $this->diffusionRequest = $drequest;
 

src/applications/diffusion/controller/DiffusionPathValidateController.php

@@ -45,19 +45,6 @@ final class DiffusionPathValidateController extends DiffusionController {
       'valid' => (bool)$valid,
     );
 
-    if (!$valid) {
-      $branch = $drequest->getBranch();
-      if ($branch) {
-        $message = pht('Not found in %s', $branch);
-      } else {
-        $message = pht('Not found at %s', 'HEAD');
-      }
-    } else {
-      $message = pht('OK');
-    }
-
-    $output['message'] = $message;
-
     return id(new AphrontAjaxResponse())->setContent($output);
   }
 }

src/applications/files/PhabricatorImageTransformer.php

@@ -6,268 +6,6 @@
  */
 final class PhabricatorImageTransformer extends Phobject {
 
-  public function executeMemeTransform(
-    PhabricatorFile $file,
-    $upper_text,
-    $lower_text) {
-    $image = $this->applyMemeToFile($file, $upper_text, $lower_text);
-    return PhabricatorFile::newFromFileData(
-      $image,
-      array(
-        'name' => 'meme-'.$file->getName(),
-        'ttl.relative' => phutil_units('24 hours in seconds'),
-        'canCDN' => true,
-      ));
-  }
-
-  public function executeConpherenceTransform(
-    PhabricatorFile $file,
-    $top,
-    $left,
-    $width,
-    $height) {
-
-    $image = $this->crasslyCropTo(
-      $file,
-      $top,
-      $left,
-      $width,
-      $height);
-
-    return PhabricatorFile::newFromFileData(
-      $image,
-      array(
-        'name' => 'conpherence-'.$file->getName(),
-        'profile' => true,
-        'canCDN' => true,
-      ));
-  }
-
-  private function crasslyCropTo(PhabricatorFile $file, $top, $left, $w, $h) {
-    $data = $file->loadFileData();
-    $src = imagecreatefromstring($data);
-    $dst = $this->getBlankDestinationFile($w, $h);
-
-    $scale = self::getScaleForCrop($file, $w, $h);
-    $orig_x = $left / $scale;
-    $orig_y = $top / $scale;
-    $orig_w = $w / $scale;
-    $orig_h = $h / $scale;
-
-    imagecopyresampled(
-      $dst,
-      $src,
-      0, 0,
-      $orig_x, $orig_y,
-      $w, $h,
-      $orig_w, $orig_h);
-
-    return self::saveImageDataInAnyFormat($dst, $file->getMimeType());
-  }
-
-  private function getBlankDestinationFile($dx, $dy) {
-    $dst = imagecreatetruecolor($dx, $dy);
-    imagesavealpha($dst, true);
-    imagefill($dst, 0, 0, imagecolorallocatealpha($dst, 255, 255, 255, 127));
-
-    return $dst;
-  }
-
-  public static function getScaleForCrop(
-    PhabricatorFile $file,
-    $des_width,
-    $des_height) {
-
-    $metadata = $file->getMetadata();
-    $width = $metadata[PhabricatorFile::METADATA_IMAGE_WIDTH];
-    $height = $metadata[PhabricatorFile::METADATA_IMAGE_HEIGHT];
-
-    if ($height < $des_height) {
-      $scale = $height / $des_height;
-    } else if ($width < $des_width) {
-      $scale = $width / $des_width;
-    } else {
-      $scale_x = $des_width / $width;
-      $scale_y = $des_height / $height;
-      $scale = max($scale_x, $scale_y);
-    }
-
-    return $scale;
-  }
-
-  private function applyMemeToFile(
-    PhabricatorFile $file,
-    $upper_text,
-    $lower_text) {
-    $data = $file->loadFileData();
-
-    $img_type = $file->getMimeType();
-    $imagemagick = PhabricatorEnv::getEnvConfig('files.enable-imagemagick');
-
-    if ($img_type != 'image/gif' || $imagemagick == false) {
-      return $this->applyMemeTo(
-        $data, $upper_text, $lower_text, $img_type);
-    }
-
-    $data = $file->loadFileData();
-    $input = new TempFile();
-    Filesystem::writeFile($input, $data);
-
-    list($out) = execx('convert %s info:', $input);
-    $split = phutil_split_lines($out);
-    if (count($split) > 1) {
-      return $this->applyMemeWithImagemagick(
-        $input,
-        $upper_text,
-        $lower_text,
-        count($split),
-        $img_type);
-    } else {
-      return $this->applyMemeTo($data, $upper_text, $lower_text, $img_type);
-    }
-  }
-
-  private function applyMemeTo(
-    $data,
-    $upper_text,
-    $lower_text,
-    $mime_type) {
-    $img = imagecreatefromstring($data);
-
-    // Some PNGs have color palettes, and allocating the dark border color
-    // fails and gives us whatever's first in the color table. Copy the image
-    // to a fresh truecolor canvas before working with it.
-
-    $truecolor = imagecreatetruecolor(imagesx($img), imagesy($img));
-    imagecopy($truecolor, $img, 0, 0, 0, 0, imagesx($img), imagesy($img));
-    $img = $truecolor;
-
-    $phabricator_root = dirname(phutil_get_library_root('phabricator'));
-    $font_root = $phabricator_root.'/resources/font/';
-    $font_path = $font_root.'tuffy.ttf';
-    if (Filesystem::pathExists($font_root.'impact.ttf')) {
-      $font_path = $font_root.'impact.ttf';
-    }
-    $text_color = imagecolorallocate($img, 255, 255, 255);
-    $border_color = imagecolorallocatealpha($img, 0, 0, 0, 110);
-    $border_width = 4;
-    $font_max = 200;
-    $font_min = 5;
-    for ($i = $font_max; $i > $font_min; $i--) {
-      $fit = $this->doesTextBoundingBoxFitInImage(
-        $img,
-        $upper_text,
-        $i,
-        $font_path);
-      if ($fit['doesfit']) {
-        $x = ($fit['imgwidth'] - $fit['txtwidth']) / 2;
-        $y = $fit['txtheight'] + 10;
-        $this->makeImageWithTextBorder($img,
-          $i,
-          $x,
-          $y,
-          $text_color,
-          $border_color,
-          $border_width,
-          $font_path,
-          $upper_text);
-        break;
-      }
-    }
-    for ($i = $font_max; $i > $font_min; $i--) {
-      $fit = $this->doesTextBoundingBoxFitInImage($img,
-        $lower_text, $i, $font_path);
-      if ($fit['doesfit']) {
-        $x = ($fit['imgwidth'] - $fit['txtwidth']) / 2;
-        $y = $fit['imgheight'] - 10;
-        $this->makeImageWithTextBorder(
-          $img,
-          $i,
-          $x,
-          $y,
-          $text_color,
-          $border_color,
-          $border_width,
-          $font_path,
-          $lower_text);
-        break;
-      }
-    }
-    return self::saveImageDataInAnyFormat($img, $mime_type);
-  }
-
-  private function makeImageWithTextBorder($img, $font_size, $x, $y,
-    $color, $stroke_color, $bw, $font, $text) {
-    $angle = 0;
-    $bw = abs($bw);
-    for ($c1 = $x - $bw; $c1 <= $x + $bw; $c1++) {
-      for ($c2 = $y - $bw; $c2 <= $y + $bw; $c2++) {
-        if (!(($c1 == $x - $bw || $x + $bw) &&
-          $c2 == $y - $bw || $c2 == $y + $bw)) {
-          $bg = imagettftext($img, $font_size,
-            $angle, $c1, $c2, $stroke_color, $font, $text);
-          }
-        }
-      }
-    imagettftext($img, $font_size, $angle,
-            $x , $y, $color , $font, $text);
-  }
-
-  private function doesTextBoundingBoxFitInImage($img,
-    $text, $font_size, $font_path) {
-    // Default Angle = 0
-    $angle = 0;
-
-    $bbox = imagettfbbox($font_size, $angle, $font_path, $text);
-    $text_height = abs($bbox[3] - $bbox[5]);
-    $text_width = abs($bbox[0] - $bbox[2]);
-    return array(
-      'doesfit' => ($text_height * 1.05 <= imagesy($img) / 2
-        && $text_width * 1.05 <= imagesx($img)),
-      'txtwidth' => $text_width,
-      'txtheight' => $text_height,
-      'imgwidth' => imagesx($img),
-      'imgheight' => imagesy($img),
-    );
-  }
-
-  private function applyMemeWithImagemagick(
-    $input,
-    $above,
-    $below,
-    $count,
-    $img_type) {
-
-    $output = new TempFile();
-    $future = new ExecFuture(
-      'convert %s -coalesce +adjoin %s_%s',
-      $input,
-      $input,
-      '%09d');
-    $future->setTimeout(10)->resolvex();
-
-    $output_files = array();
-    for ($ii = 0; $ii < $count; $ii++) {
-      $frame_name = sprintf('%s_%09d', $input, $ii);
-      $output_name = sprintf('%s_%09d', $output, $ii);
-
-      $output_files[] = $output_name;
-
-      $frame_data = Filesystem::readFile($frame_name);
-      $memed_frame_data = $this->applyMemeTo(
-        $frame_data,
-        $above,
-        $below,
-        $img_type);
-      Filesystem::writeFile($output_name, $memed_frame_data);
-    }
-
-    $future = new ExecFuture('convert -loop 0 %Ls %s', $output_files, $output);
-    $future->setTimeout(10)->resolvex();
-
-    return Filesystem::readFile($output);
-  }
-
 
 /* -(  Saving Image Data  )-------------------------------------------------- */
 

src/applications/files/controller/PhabricatorFileImageProxyController.php

@@ -8,15 +8,6 @@ final class PhabricatorFileImageProxyController
   }
 
   public function handleRequest(AphrontRequest $request) {
-
-    $show_prototypes = PhabricatorEnv::getEnvConfig(
-      'phabricator.show-prototypes');
-    if (!$show_prototypes) {
-      throw new Exception(
-        pht('Show prototypes is disabled.
-          Set `phabricator.show-prototypes` to `true` to use the image proxy'));
-    }
-
     $viewer = $request->getViewer();
     $img_uri = $request->getStr('uri');
 
@@ -24,9 +15,16 @@ final class PhabricatorFileImageProxyController
     PhabricatorEnv::requireValidRemoteURIForLink($img_uri);
     $uri = new PhutilURI($img_uri);
     $proto = $uri->getProtocol();
-    if (!in_array($proto, array('http', 'https'))) {
+
+    $allowed_protocols = array(
+      'http',
+      'https',
+    );
+    if (!in_array($proto, $allowed_protocols)) {
       throw new Exception(
-        pht('The provided image URI must be either http or https'));
+        pht(
+          'The provided image URI must use one of these protocols: %s.',
+          implode(', ', $allowed_protocols)));
     }
 
     // Check if we already have the specified image URI downloaded
@@ -43,8 +41,9 @@ final class PhabricatorFileImageProxyController
       ->setURI($img_uri)
       ->setTTL($ttl);
 
+    // Cache missed, so we'll need to validate and download the image.
     $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
-    // Cache missed so we'll need to validate and download the image
+    $save_request = false;
     try {
       // Rate limit outbound fetches to make this mechanism less useful for
       // scanning networks and ports.
@@ -59,6 +58,7 @@ final class PhabricatorFileImageProxyController
           'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
           'canCDN' => true,
         ));
+
       if (!$file->isViewableImage()) {
         $mime_type = $file->getMimeType();
         $engine = new PhabricatorDestructionEngine();
@@ -66,53 +66,82 @@ final class PhabricatorFileImageProxyController
         $file = null;
         throw new Exception(
           pht(
-            'The URI "%s" does not correspond to a valid image file, got '.
+            'The URI "%s" does not correspond to a valid image file (got '.
-            'a file with MIME type "%s". You must specify the URI of a '.
+            'a file with MIME type "%s"). You must specify the URI of a '.
             'valid image file.',
             $uri,
             $mime_type));
-      } else {
-        $file->save();
       }
 
-      $external_request->setIsSuccessful(true)
+      $file->save();
-        ->setFilePHID($file->getPHID())
+
-        ->save();
+      $external_request
-      unset($unguarded);
+        ->setIsSuccessful(1)
-      return $this->getExternalResponse($external_request);
+        ->setFilePHID($file->getPHID());
+
+      $save_request = true;
     } catch (HTTPFutureHTTPResponseStatus $status) {
-      $external_request->setIsSuccessful(false)
+      $external_request
-        ->setResponseMessage($status->getMessage())
+        ->setIsSuccessful(0)
-        ->save();
+        ->setResponseMessage($status->getMessage());
-      return $this->getExternalResponse($external_request);
+
+      $save_request = true;
     } catch (Exception $ex) {
       // Not actually saving the request in this case
       $external_request->setResponseMessage($ex->getMessage());
-      return $this->getExternalResponse($external_request);
     }
+
+    if ($save_request) {
+      try {
+        $external_request->save();
+      } catch (AphrontDuplicateKeyQueryException $ex) {
+        // We may have raced against another identical request. If we did,
+        // just throw our result away and use the winner's result.
+        $external_request = $external_request->loadOneWhere(
+          'uriIndex = %s',
+          PhabricatorHash::digestForIndex($img_uri));
+        if (!$external_request) {
+          throw new Exception(
+            pht(
+              'Hit duplicate key collision when saving proxied image, but '.
+              'failed to load duplicate row (for URI "%s").',
+              $img_uri));
+        }
+      }
+    }
+
+    unset($unguarded);
+
+
+    return $this->getExternalResponse($external_request);
   }
 
   private function getExternalResponse(
     PhabricatorFileExternalRequest $request) {
-    if ($request->getIsSuccessful()) {
+    if (!$request->getIsSuccessful()) {
+      throw new Exception(
+        pht(
+          'Request to "%s" failed: %s',
+          $request->getURI(),
+          $request->getResponseMessage()));
+    }
+
     $file = id(new PhabricatorFileQuery())
       ->setViewer(PhabricatorUser::getOmnipotentUser())
       ->withPHIDs(array($request->getFilePHID()))
       ->executeOne();
     if (!$file) {
-        throw new Exception(pht(
+      throw new Exception(
+        pht(
           'The underlying file does not exist, but the cached request was '.
-          'successful. This likely means the file record was manually deleted '.
+          'successful. This likely means the file record was manually '.
-          'by an administrator.'));
+          'deleted by an administrator.'));
-      }
-      return id(new AphrontRedirectResponse())
-        ->setIsExternal(true)
-        ->setURI($file->getViewURI());
-    } else {
-      throw new Exception(pht(
-        "The request to get the external file from '%s' was unsuccessful:\n %s",
-        $request->getURI(),
-        $request->getResponseMessage()));
     }
+
+    return id(new AphrontAjaxResponse())
+      ->setContent(
+        array(
+          'imageURI' => $file->getViewURI(),
+        ));
   }
 }

src/applications/files/markup/PhabricatorImageRemarkupRule.php

@@ -1,6 +1,9 @@
 <?php
 
 final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule {
+
+  const KEY_RULE_EXTERNAL_IMAGE = 'rule.external-image';
+
   public function getPriority() {
     return 200.0;
   }
@@ -16,6 +19,7 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule {
     if (!$this->isFlatText($matches[0])) {
       return $matches[0];
     }
+
     $args = array();
     $defaults = array(
       'uri' => null,
@@ -23,9 +27,10 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule {
       'width' => null,
       'height' => null,
     );
+
     $trimmed_match = trim($matches[2]);
     if ($this->isURI($trimmed_match)) {
-      $args['uri'] = new PhutilURI($trimmed_match);
+      $args['uri'] = $trimmed_match;
     } else {
       $parser = new PhutilSimpleOptions();
       $keys = $parser->parse($trimmed_match);
@@ -37,27 +42,123 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule {
         }
       }
       if ($uri_key) {
-        $args['uri'] = new PhutilURI($keys[$uri_key]);
+        $args['uri'] = $keys[$uri_key];
       }
       $args += $keys;
     }
 
     $args += $defaults;
 
-    if ($args['uri']) {
+    if (!strlen($args['uri'])) {
-      $src_uri = id(new PhutilURI('/file/imageproxy/'))
+      return $matches[0];
-        ->setQueryParam('uri', (string)$args['uri']);
+    }
-      $img = $this->newTag(
+
+    // Make sure this is something that looks roughly like a real URI. We'll
+    // validate it more carefully before proxying it, but if whatever the user
+    // has typed isn't even close, just decline to activate the rule behavior.
+    try {
+      $uri = new PhutilURI($args['uri']);
+
+      if (!strlen($uri->getProtocol())) {
+        return $matches[0];
+      }
+
+      $args['uri'] = (string)$uri;
+    } catch (Exception $ex) {
+      return $matches[0];
+    }
+
+    $engine = $this->getEngine();
+    $metadata_key = self::KEY_RULE_EXTERNAL_IMAGE;
+    $metadata = $engine->getTextMetadata($metadata_key, array());
+
+    $token = $engine->storeText('<img>');
+
+    $metadata[] = array(
+      'token' => $token,
+      'args' => $args,
+    );
+
+    $engine->setTextMetadata($metadata_key, $metadata);
+
+    return $token;
+  }
+
+  public function didMarkupText() {
+    $engine = $this->getEngine();
+    $metadata_key = self::KEY_RULE_EXTERNAL_IMAGE;
+    $images = $engine->getTextMetadata($metadata_key, array());
+    $engine->setTextMetadata($metadata_key, array());
+
+    if (!$images) {
+      return;
+    }
+
+    // Look for images we've already successfully fetched that aren't about
+    // to get eaten by the GC. For any we find, we can just emit a normal
+    // "<img />" tag pointing directly to the file.
+
+    // For files which we don't hit in the cache, we emit a placeholder
+    // instead and use AJAX to actually perform the fetch.
+
+    $digests = array();
+    foreach ($images as $image) {
+      $uri = $image['args']['uri'];
+      $digests[] = PhabricatorHash::digestForIndex($uri);
+    }
+
+    $caches = id(new PhabricatorFileExternalRequest())->loadAllWhere(
+      'uriIndex IN (%Ls) AND isSuccessful = 1 AND ttl > %d',
+      $digests,
+      PhabricatorTime::getNow() + phutil_units('1 hour in seconds'));
+
+    $file_phids = array();
+    foreach ($caches as $cache) {
+      $file_phids[$cache->getFilePHID()] = $cache->getURI();
+    }
+
+    $file_map = array();
+    if ($file_phids) {
+      $files = id(new PhabricatorFileQuery())
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
+        ->withPHIDs(array_keys($file_phids))
+        ->execute();
+      foreach ($files as $file) {
+        $phid = $file->getPHID();
+
+        $file_remote_uri = $file_phids[$phid];
+        $file_view_uri = $file->getViewURI();
+
+        $file_map[$file_remote_uri] = $file_view_uri;
+      }
+    }
+
+    foreach ($images as $image) {
+      $args = $image['args'];
+      $uri = $args['uri'];
+
+      $direct_uri = idx($file_map, $uri);
+      if ($direct_uri) {
+        $img = phutil_tag(
           'img',
           array(
-          'src' => $src_uri,
+            'src' => $direct_uri,
             'alt' => $args['alt'],
             'width' => $args['width'],
             'height' => $args['height'],
           ));
-      return $this->getEngine()->storeText($img);
       } else {
-      return $matches[0];
+        $src_uri = id(new PhutilURI('/file/imageproxy/'))
+          ->setQueryParam('uri', $uri);
+
+        $img = id(new PHUIRemarkupImageView())
+          ->setURI($src_uri)
+          ->setAlt($args['alt'])
+          ->setWidth($args['width'])
+          ->setHeight($args['height']);
+      }
+
+      $engine->overwriteStoredText($image['token'], $img);
     }
   }
 
@@ -66,4 +167,5 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule {
     // If it does, we'll try to treat it like a valid URI
     return preg_match('~^https?\:\/\/.*\z~i', $uri_string);
   }
+
 }

src/applications/macro/conduit/MacroCreateMemeConduitAPIMethod.php

@@ -35,22 +35,15 @@ final class MacroCreateMemeConduitAPIMethod extends MacroConduitAPIMethod {
   protected function execute(ConduitAPIRequest $request) {
     $user = $request->getUser();
 
-    $macro_name = $request->getValue('macroName');
+    $file = id(new PhabricatorMemeEngine())
-    $upper_text = $request->getValue('upperText');
+      ->setViewer($user)
-    $lower_text = $request->getValue('lowerText');
+      ->setTemplate($request->getValue('macroName'))
-
+      ->setAboveText($request->getValue('upperText'))
-    $uri = PhabricatorMacroMemeController::generateMacro(
+      ->setBelowText($request->getValue('lowerText'))
-      $user,
+      ->newAsset();
-      $macro_name,
-      $upper_text,
-      $lower_text);
-
-    if (!$uri) {
-      throw new ConduitException('ERR-NOT-FOUND');
-    }
 
     return array(
-      'uri' => $uri,
+      'uri' => $file->getViewURI(),
     );
   }
 

src/applications/macro/controller/PhabricatorMacroMemeController.php

@@ -13,56 +13,18 @@ final class PhabricatorMacroMemeController
     $lower_text = $request->getStr('lowertext');
     $viewer = $request->getViewer();
 
-    $uri = self::generateMacro($viewer, $macro_name,
+    $file = id(new PhabricatorMemeEngine())
-      $upper_text, $lower_text);
-    if ($uri === false) {
-      return new Aphront404Response();
-    }
-    return id(new AphrontRedirectResponse())
-      ->setIsExternal(true)
-      ->setURI($uri);
-  }
-
-  public static function generateMacro($viewer, $macro_name, $upper_text,
-      $lower_text) {
-    $macro = id(new PhabricatorMacroQuery())
       ->setViewer($viewer)
-      ->withNames(array($macro_name))
+      ->setTemplate($macro_name)
-      ->needFiles(true)
+      ->setAboveText($request->getStr('above'))
-      ->executeOne();
+      ->setBelowText($request->getStr('below'))
-    if (!$macro) {
+      ->newAsset();
-      return false;
-    }
-    $file = $macro->getFile();
 
-    $upper_text = strtoupper($upper_text);
+    $content = array(
-    $lower_text = strtoupper($lower_text);
+      'imageURI' => $file->getViewURI(),
-    $mixed_text = md5($upper_text).':'.md5($lower_text);
+    );
-    $hash = 'meme'.hash('sha256', $mixed_text);
-    $xform = id(new PhabricatorTransformedFile())
-      ->loadOneWhere('originalphid=%s and transform=%s',
-        $file->getPHID(), $hash);
 
-    if ($xform) {
+    return id(new AphrontAjaxResponse())->setContent($content);
-      $memefile = id(new PhabricatorFileQuery())
-        ->setViewer($viewer)
-        ->withPHIDs(array($xform->getTransformedPHID()))
-        ->executeOne();
-      if ($memefile) {
-        return $memefile->getBestURI();
-      }
   }
 
-    $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
-    $transformers = (new PhabricatorImageTransformer());
-    $newfile = $transformers
-      ->executeMemeTransform($file, $upper_text, $lower_text);
-    $xfile = new PhabricatorTransformedFile();
-    $xfile->setOriginalPHID($file->getPHID());
-    $xfile->setTransformedPHID($newfile->getPHID());
-    $xfile->setTransform($hash);
-    $xfile->save();
-
-    return $newfile->getBestURI();
-  }
 }

src/applications/macro/engine/PhabricatorMemeEngine.php

@@ -0,0 +1,384 @@
+<?php
+
+final class PhabricatorMemeEngine extends Phobject {
+
+  private $viewer;
+  private $template;
+  private $aboveText;
+  private $belowText;
+
+  private $templateFile;
+  private $metrics;
+
+  public function setViewer(PhabricatorUser $viewer) {
+    $this->viewer = $viewer;
+    return $this;
+  }
+
+  public function getViewer() {
+    return $this->viewer;
+  }
+
+  public function setTemplate($template) {
+    $this->template = $template;
+    return $this;
+  }
+
+  public function getTemplate() {
+    return $this->template;
+  }
+
+  public function setAboveText($above_text) {
+    $this->aboveText = $above_text;
+    return $this;
+  }
+
+  public function getAboveText() {
+    return $this->aboveText;
+  }
+
+  public function setBelowText($below_text) {
+    $this->belowText = $below_text;
+    return $this;
+  }
+
+  public function getBelowText() {
+    return $this->belowText;
+  }
+
+  public function getGenerateURI() {
+    return id(new PhutilURI('/macro/meme/'))
+      ->alter('macro', $this->getTemplate())
+      ->alter('above', $this->getAboveText())
+      ->alter('below', $this->getBelowText());
+  }
+
+  public function newAsset() {
+    $cache = $this->loadCachedFile();
+    if ($cache) {
+      return $cache;
+    }
+
+    $template = $this->loadTemplateFile();
+    if (!$template) {
+      throw new Exception(
+        pht(
+          'Template "%s" is not a valid template.',
+          $template));
+    }
+
+    $hash = $this->newTransformHash();
+
+    $asset = $this->newAssetFile($template);
+
+    $xfile = id(new PhabricatorTransformedFile())
+      ->setOriginalPHID($template->getPHID())
+      ->setTransformedPHID($asset->getPHID())
+      ->setTransform($hash);
+
+    try {
+      $caught = null;
+
+      $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+      try {
+        $xfile->save();
+      } catch (Exception $ex) {
+        $caught = $ex;
+      }
+      unset($unguarded);
+
+      if ($caught) {
+        throw $caught;
+      }
+
+      return $asset;
+    } catch (AphrontDuplicateKeyQueryException $ex) {
+      $xfile = $this->loadCachedFile();
+      if (!$xfile) {
+        throw $ex;
+      }
+      return $xfile;
+    }
+  }
+
+  private function newTransformHash() {
+    $properties = array(
+      'kind' => 'meme',
+      'above' => phutil_utf8_strtoupper($this->getAboveText()),
+      'below' => phutil_utf8_strtoupper($this->getBelowText()),
+    );
+
+    $properties = phutil_json_encode($properties);
+
+    return PhabricatorHash::digestForIndex($properties);
+  }
+
+  public function loadCachedFile() {
+    $viewer = $this->getViewer();
+
+    $template_file = $this->loadTemplateFile();
+    if (!$template_file) {
+      return null;
+    }
+
+    $hash = $this->newTransformHash();
+
+    $xform = id(new PhabricatorTransformedFile())->loadOneWhere(
+      'originalPHID = %s AND transform = %s',
+      $template_file->getPHID(),
+      $hash);
+    if (!$xform) {
+      return null;
+    }
+
+    return id(new PhabricatorFileQuery())
+      ->setViewer($viewer)
+      ->withPHIDs(array($xform->getTransformedPHID()))
+      ->executeOne();
+  }
+
+  private function loadTemplateFile() {
+    if ($this->templateFile === null) {
+      $viewer = $this->getViewer();
+      $template = $this->getTemplate();
+
+      $macro = id(new PhabricatorMacroQuery())
+        ->setViewer($viewer)
+        ->withNames(array($template))
+        ->needFiles(true)
+        ->executeOne();
+      if (!$macro) {
+        return null;
+      }
+
+      $this->templateFile = $macro->getFile();
+    }
+
+    return $this->templateFile;
+  }
+
+  private function newAssetFile(PhabricatorFile $template) {
+    $data = $this->newAssetData($template);
+    return PhabricatorFile::newFromFileData(
+      $data,
+      array(
+        'name' => 'meme-'.$template->getName(),
+        'canCDN' => true,
+
+        // In modern code these can end up linked directly in email, so let
+        // them stick around for a while.
+        'ttl.relative' => phutil_units('30 days in seconds'),
+      ));
+  }
+
+  private function newAssetData(PhabricatorFile $template) {
+    $template_data = $template->loadFileData();
+
+    $result = $this->newImagemagickAsset($template, $template_data);
+    if ($result) {
+      return $result;
+    }
+
+    return $this->newGDAsset($template, $template_data);
+  }
+
+  private function newImagemagickAsset(
+    PhabricatorFile $template,
+    $template_data) {
+
+    // We're only going to use Imagemagick on GIFs.
+    $mime_type = $template->getMimeType();
+    if ($mime_type != 'image/gif') {
+      return null;
+    }
+
+    // We're only going to use Imagemagick if it is actually available.
+    $available = PhabricatorEnv::getEnvConfig('files.enable-imagemagick');
+    if (!$available) {
+      return null;
+    }
+
+    // Test of the GIF is an animated GIF. If it's a flat GIF, we'll fall
+    // back to GD.
+    $input = new TempFile();
+    Filesystem::writeFile($input, $template_data);
+    list($err, $out) = exec_manual('convert %s info:', $input);
+    if ($err) {
+      return null;
+    }
+
+    $split = phutil_split_lines($out);
+    $frames = count($split);
+    if ($frames <= 1) {
+      return null;
+    }
+
+    // Split the frames apart, transform each frame, then merge them back
+    // together.
+    $output = new TempFile();
+
+    $future = new ExecFuture(
+      'convert %s -coalesce +adjoin %s_%s',
+      $input,
+      $input,
+      '%09d');
+    $future->setTimeout(10)->resolvex();
+
+    $output_files = array();
+    for ($ii = 0; $ii < $frames; $ii++) {
+      $frame_name = sprintf('%s_%09d', $input, $ii);
+      $output_name = sprintf('%s_%09d', $output, $ii);
+
+      $output_files[] = $output_name;
+
+      $frame_data = Filesystem::readFile($frame_name);
+      $memed_frame_data = $this->newGDAsset($template, $frame_data);
+      Filesystem::writeFile($output_name, $memed_frame_data);
+    }
+
+    $future = new ExecFuture('convert -loop 0 %Ls %s', $output_files, $output);
+    $future->setTimeout(10)->resolvex();
+
+    return Filesystem::readFile($output);
+  }
+
+  private function newGDAsset(PhabricatorFile $template, $data) {
+    $img = imagecreatefromstring($data);
+    if (!$img) {
+      throw new Exception(
+        pht('Failed to imagecreatefromstring() image template data.'));
+    }
+
+    $dx = imagesx($img);
+    $dy = imagesy($img);
+
+    $metrics = $this->getMetrics($dx, $dy);
+    $font = $this->getFont();
+    $size = $metrics['size'];
+
+    $above = $this->getAboveText();
+    if (strlen($above)) {
+      $x = (int)floor(($dx - $metrics['text']['above']['width']) / 2);
+      $y = $metrics['text']['above']['height'] + 12;
+
+      $this->drawText($img, $font, $metrics['size'], $x, $y, $above);
+    }
+
+    $below = $this->getBelowText();
+    if (strlen($below)) {
+      $x = (int)floor(($dx - $metrics['text']['below']['width']) / 2);
+      $y = $dy - 12 - $metrics['text']['below']['descend'];
+
+      $this->drawText($img, $font, $metrics['size'], $x, $y, $below);
+    }
+
+    return PhabricatorImageTransformer::saveImageDataInAnyFormat(
+      $img,
+      $template->getMimeType());
+  }
+
+  private function getFont() {
+    $phabricator_root = dirname(phutil_get_library_root('phabricator'));
+
+    $font_root = $phabricator_root.'/resources/font/';
+    if (Filesystem::pathExists($font_root.'impact.ttf')) {
+      $font_path = $font_root.'impact.ttf';
+    } else {
+      $font_path = $font_root.'tuffy.ttf';
+    }
+
+    return $font_path;
+  }
+
+  private function getMetrics($dim_x, $dim_y) {
+    if ($this->metrics === null) {
+      $font = $this->getFont();
+
+      $font_max = 72;
+      $font_min = 5;
+
+      $last = null;
+      $cursor = floor(($font_max + $font_min) / 2);
+      $min = $font_min;
+      $max = $font_max;
+
+      $texts = array(
+        'above' => $this->getAboveText(),
+        'below' => $this->getBelowText(),
+      );
+
+      $metrics = null;
+      $best = null;
+      while (true) {
+        $all_fit = true;
+        $text_metrics = array();
+        foreach ($texts as $key => $text) {
+          $box = imagettfbbox($cursor, 0, $font, $text);
+          $height = abs($box[3] - $box[5]);
+          $width = abs($box[0] - $box[2]);
+
+          // This is the number of pixels below the baseline that the
+          // text extends, for example if it has a "y".
+          $descend = $box[3];
+
+          if ($height > $dim_y) {
+            $all_fit = false;
+            break;
+          }
+
+          if ($width > $dim_x) {
+            $all_fit = false;
+            break;
+          }
+
+          $text_metrics[$key]['width'] = $width;
+          $text_metrics[$key]['height'] = $height;
+          $text_metrics[$key]['descend'] = $descend;
+        }
+
+        if ($all_fit || $best === null) {
+          $best = $cursor;
+          $metrics = $text_metrics;
+        }
+
+        if ($all_fit) {
+          $min = $cursor;
+        } else {
+          $max = $cursor;
+        }
+
+        $last = $cursor;
+        $cursor = floor(($max + $min) / 2);
+        if ($cursor === $last) {
+          break;
+        }
+      }
+
+      $this->metrics = array(
+        'size' => $best,
+        'text' => $metrics,
+      );
+    }
+
+    return $this->metrics;
+  }
+
+  private function drawText($img, $font, $size, $x, $y, $text) {
+    $text_color = imagecolorallocate($img, 255, 255, 255);
+    $border_color = imagecolorallocate($img, 0, 0, 0);
+
+    $border = 2;
+    for ($xx = ($x - $border); $xx <= ($x + $border); $xx += $border) {
+      for ($yy = ($y - $border); $yy <= ($y + $border); $yy += $border) {
+        if (($xx === $x) && ($yy === $y)) {
+          continue;
+        }
+        imagettftext($img, $size, 0, $xx, $yy, $border_color, $font, $text);
+      }
+    }
+
+    imagettftext($img, $size, 0, $x, $y, $text_color, $font, $text);
+  }
+
+
+}

src/applications/macro/markup/PhabricatorMemeRemarkupRule.php

@@ -29,34 +29,72 @@ final class PhabricatorMemeRemarkupRule extends PhutilRemarkupRule {
     $parser = new PhutilSimpleOptions();
     $options = $parser->parse($matches[1]) + $options;
 
-    $uri = id(new PhutilURI('/macro/meme/'))
+    $engine = id(new PhabricatorMemeEngine())
-      ->alter('macro', $options['src'])
+      ->setViewer(PhabricatorUser::getOmnipotentUser())
-      ->alter('uppertext', $options['above'])
+      ->setTemplate($options['src'])
-      ->alter('lowertext', $options['below']);
+      ->setAboveText($options['above'])
+      ->setBelowText($options['below']);
 
-    if ($this->getEngine()->isHTMLMailMode()) {
+    $asset = $engine->loadCachedFile();
-      $uri = PhabricatorEnv::getProductionURI($uri);
+
+    $is_html_mail = $this->getEngine()->isHTMLMailMode();
+    $is_text = $this->getEngine()->isTextMode();
+    $must_inline = ($is_html_mail || $is_text);
+
+    if ($must_inline) {
+      if (!$asset) {
+        try {
+          $asset = $engine->newAsset();
+        } catch (Exception $ex) {
+          return $matches[0];
+        }
+      }
     }
 
-    if ($this->getEngine()->isTextMode()) {
+    if ($asset) {
-      $img =
+      $uri = $asset->getViewURI();
-        ($options['above'] != '' ? "\"{$options['above']}\"\n" : '').
-        $options['src'].' <'.PhabricatorEnv::getProductionURI($uri).'>'.
-        ($options['below'] != '' ? "\n\"{$options['below']}\"" : '');
     } else {
+      $uri = $engine->getGenerateURI();
+    }
+
+    if ($is_text) {
+      $parts = array();
+
+      $above = $options['above'];
+      if (strlen($above)) {
+        $parts[] = pht('"%s"', $above);
+      }
+
+      $parts[] = $options['src'].' <'.$uri.'>';
+
+      $below = $options['below'];
+      if (strlen($below)) {
+        $parts[] = pht('"%s"', $below);
+      }
+
+      $parts = implode("\n", $parts);
+      return $this->getEngine()->storeText($parts);
+    }
+
     $alt_text = pht(
       'Macro %s: %s %s',
       $options['src'],
       $options['above'],
       $options['below']);
 
+    if ($asset) {
       $img = $this->newTag(
         'img',
         array(
           'src' => $uri,
-          'alt' => $alt_text,
           'class' => 'phabricator-remarkup-macro',
+          'alt' => $alt_text,
         ));
+    } else {
+      $img = id(new PHUIRemarkupImageView())
+        ->setURI($uri)
+        ->addClass('phabricator-remarkup-macro')
+        ->setAlt($alt_text);
     }
 
     return $this->getEngine()->storeText($img);

src/applications/maniphest/storage/ManiphestTransaction.php

@@ -40,22 +40,6 @@ final class ManiphestTransaction
     return parent::shouldGenerateOldValue();
   }
 
-  public function shouldHideForFeed() {
-    // NOTE: Modular transactions don't currently support this, and it has
-    // very few callsites, and it's publish-time rather than display-time.
-    // This should probably become a supported, display-time behavior. For
-    // discussion, see T12787.
-
-    // Hide "alice created X, a task blocking Y." from feed because it
-    // will almost always appear adjacent to "alice created Y".
-    $is_new = $this->getMetadataValue('blocker.new');
-    if ($is_new) {
-      return true;
-    }
-
-    return parent::shouldHideForFeed();
-  }
-
   public function getRequiredHandlePHIDs() {
     $phids = parent::getRequiredHandlePHIDs();
 

src/applications/maniphest/xaction/ManiphestTaskUnblockTransaction.php

@@ -112,5 +112,15 @@ final class ManiphestTaskUnblockTransaction
     return 'fa-shield';
   }
 
+  public function shouldHideForFeed() {
+    // Hide "alice created X, a task blocking Y." from feed because it
+    // will almost always appear adjacent to "alice created Y".
+    $is_new = $this->getMetadataValue('blocker.new');
+    if ($is_new) {
+      return true;
+    }
+
+    return parent::shouldHideForFeed();
+  }
 
 }

src/applications/owners/controller/PhabricatorOwnersDetailController.php

@@ -279,7 +279,7 @@ final class PhabricatorOwnersDetailController
       $href = $repo->generateURI(
         array(
           'branch'   => $repo->getDefaultBranch(),
-          'path'     => $path->getPath(),
+          'path'     => $path->getPathDisplay(),
           'action'   => 'browse',
         ));
 
@@ -288,7 +288,7 @@ final class PhabricatorOwnersDetailController
         array(
           'href' => (string)$href,
         ),
-        $path->getPath());
+        $path->getPathDisplay());
 
       $rows[] = array(
         ($path->getExcluded() ? '-' : '+'),

src/applications/owners/controller/PhabricatorOwnersPathsController.php

@@ -27,7 +27,7 @@ final class PhabricatorOwnersPathsController
 
       $path_refs = array();
       foreach ($paths as $key => $path) {
-        if (!isset($repos[$key])) {
+        if (!isset($repos[$key]) || !strlen($repos[$key])) {
           throw new Exception(
             pht(
               'No repository PHID for path "%s"!',
@@ -70,26 +70,39 @@ final class PhabricatorOwnersPathsController
       $path_refs = mpull($paths, 'getRef');
     }
 
-    $repos = id(new PhabricatorRepositoryQuery())
+    $template = new AphrontTokenizerTemplateView();
-      ->setViewer($viewer)
-      ->execute();
 
-    $default_paths = array();
+    $datasource = id(new DiffusionRepositoryDatasource())
-    foreach ($repos as $repo) {
+      ->setViewer($viewer);
-      $default_path = $repo->getDetail('default-owners-path');
+
-      if ($default_path) {
+    $tokenizer_spec = array(
-        $default_paths[$repo->getPHID()] = $default_path;
+      'markup' => $template->render(),
-      }
+      'config' => array(
+        'src' => $datasource->getDatasourceURI(),
+        'browseURI' => $datasource->getBrowseURI(),
+        'placeholder' => $datasource->getPlaceholderText(),
+        'limit' => 1,
+      ),
+    );
+
+    foreach ($path_refs as $key => $path_ref) {
+      $path_refs[$key]['repositoryValue'] = $datasource->getWireTokens(
+        array(
+          $path_ref['repositoryPHID'],
+        ));
     }
 
+    $icon_test = id(new PHUIIconView())
+      ->setIcon('fa-spinner grey')
+      ->setTooltip(pht('Validating...'));
 
-    $repo_map = array();
+    $icon_okay = id(new PHUIIconView())
-    foreach ($repos as $key => $repo) {
+      ->setIcon('fa-check-circle green')
-      $monogram = $repo->getMonogram();
+      ->setTooltip(pht('Path Exists in Repository'));
-      $name = $repo->getName();
+
-      $repo_map[$repo->getPHID()] = "{$monogram} {$name}";
+    $icon_fail = id(new PHUIIconView())
-    }
+      ->setIcon('fa-question-circle-o red')
-    asort($repos);
+      ->setTooltip(pht('Path Not Found On Default Branch'));
 
     $template = new AphrontTypeaheadTemplateView();
     $template = $template->render();
@@ -100,14 +113,20 @@ final class PhabricatorOwnersPathsController
         'root' => 'path-editor',
         'table' => 'paths',
         'add_button' => 'addpath',
-        'repositories'        => $repo_map,
         'input_template' => $template,
         'pathRefs' => $path_refs,
-
         'completeURI' => '/diffusion/services/path/complete/',
         'validateURI' => '/diffusion/services/path/validate/',
-
+        'repositoryTokenizerSpec' => $tokenizer_spec,
-        'repositoryDefaultPaths' => $default_paths,
+        'icons' => array(
+          'test' => hsprintf('%s', $icon_test),
+          'okay' => hsprintf('%s', $icon_okay),
+          'fail' => hsprintf('%s', $icon_fail),
+        ),
+        'modeOptions' => array(
+          0 => pht('Include'),
+          1 => pht('Exclude'),
+        ),
       ));
 
     require_celerity_resource('owners-path-editor-css');

src/applications/owners/engineextension/PhabricatorOwnersPathsSearchEngineAttachment.php

@@ -22,7 +22,7 @@ final class PhabricatorOwnersPathsSearchEngineAttachment
     foreach ($paths as $path) {
       $list[] = array(
         'repositoryPHID' => $path->getRepositoryPHID(),
-        'path' => $path->getPath(),
+        'path' => $path->getPathDisplay(),
         'excluded' => (bool)$path->getExcluded(),
       );
     }

src/applications/owners/query/PhabricatorOwnersPackageQuery.php

@@ -206,8 +206,8 @@ final class PhabricatorOwnersPackageQuery
     if ($this->paths !== null) {
       $where[] = qsprintf(
         $conn,
-        'rpath.path IN (%Ls)',
+        'rpath.pathIndex IN (%Ls)',
-        $this->getFragmentsForPaths($this->paths));
+        $this->getFragmentIndexesForPaths($this->paths));
     }
 
     if ($this->statuses !== null) {
@@ -220,13 +220,13 @@ final class PhabricatorOwnersPackageQuery
     if ($this->controlMap) {
       $clauses = array();
       foreach ($this->controlMap as $repository_phid => $paths) {
-        $fragments = $this->getFragmentsForPaths($paths);
+        $indexes = $this->getFragmentIndexesForPaths($paths);
 
         $clauses[] = qsprintf(
           $conn,
-          '(rpath.repositoryPHID = %s AND rpath.path IN (%Ls))',
+          '(rpath.repositoryPHID = %s AND rpath.pathIndex IN (%Ls))',
           $repository_phid,
-          $fragments);
+          $indexes);
       }
       $where[] = implode(' OR ', $clauses);
     }
@@ -333,6 +333,16 @@ final class PhabricatorOwnersPackageQuery
     return $fragments;
   }
 
+  private function getFragmentIndexesForPaths(array $paths) {
+    $indexes = array();
+
+    foreach ($this->getFragmentsForPaths($paths) as $fragment) {
+      $indexes[] = PhabricatorHash::digestForIndex($fragment);
+    }
+
+    return $indexes;
+  }
+
 
 /* -(  Path Control  )------------------------------------------------------- */
 

src/applications/owners/storage/PhabricatorOwnersPackage.php

@@ -16,7 +16,6 @@ final class PhabricatorOwnersPackage
   protected $auditingEnabled;
   protected $autoReview;
   protected $description;
-  protected $primaryOwnerPHID;
   protected $mailKey;
   protected $status;
   protected $viewPolicy;
@@ -33,8 +32,11 @@ final class PhabricatorOwnersPackage
 
   const AUTOREVIEW_NONE = 'none';
   const AUTOREVIEW_SUBSCRIBE = 'subscribe';
+  const AUTOREVIEW_SUBSCRIBE_ALWAYS = 'subscribe-always';
   const AUTOREVIEW_REVIEW = 'review';
+  const AUTOREVIEW_REVIEW_ALWAYS = 'review-always';
   const AUTOREVIEW_BLOCK = 'block';
+  const AUTOREVIEW_BLOCK_ALWAYS = 'block-always';
 
   const DOMINION_STRONG = 'strong';
   const DOMINION_WEAK = 'weak';
@@ -74,14 +76,26 @@ final class PhabricatorOwnersPackage
       self::AUTOREVIEW_NONE => array(
         'name' => pht('No Autoreview'),
       ),
-      self::AUTOREVIEW_SUBSCRIBE => array(
-        'name' => pht('Subscribe to Changes'),
-      ),
       self::AUTOREVIEW_REVIEW => array(
-        'name' => pht('Review Changes'),
+        'name' => pht('Review Changes With Non-Owner Author'),
+        'authority' => true,
       ),
       self::AUTOREVIEW_BLOCK => array(
-        'name' => pht('Review Changes (Blocking)'),
+        'name' => pht('Review Changes With Non-Owner Author (Blocking)'),
+        'authority' => true,
+      ),
+      self::AUTOREVIEW_SUBSCRIBE => array(
+        'name' => pht('Subscribe to Changes With Non-Owner Author'),
+        'authority' => true,
+      ),
+      self::AUTOREVIEW_REVIEW_ALWAYS => array(
+        'name' => pht('Review All Changes'),
+      ),
+      self::AUTOREVIEW_BLOCK_ALWAYS => array(
+        'name' => pht('Review All Changes (Blocking)'),
+      ),
+      self::AUTOREVIEW_SUBSCRIBE_ALWAYS => array(
+        'name' => pht('Subscribe to All Changes'),
       ),
     );
   }
@@ -107,7 +121,6 @@ final class PhabricatorOwnersPackage
       self::CONFIG_COLUMN_SCHEMA => array(
         'name' => 'sort',
         'description' => 'text',
-        'primaryOwnerPHID' => 'phid?',
         'auditingEnabled' => 'bool',
         'mailKey' => 'bytes20',
         'status' => 'text32',
@@ -203,15 +216,20 @@ final class PhabricatorOwnersPackage
     // and then merge results in PHP.
 
     $rows = array();
-    foreach (array_chunk(array_keys($fragments), 128) as $chunk) {
+    foreach (array_chunk(array_keys($fragments), 1024) as $chunk) {
+      $indexes = array();
+      foreach ($chunk as $fragment) {
+        $indexes[] = PhabricatorHash::digestForIndex($fragment);
+      }
+
       $rows[] = queryfx_all(
         $conn,
         'SELECT pkg.id, pkg.dominion, p.excluded, p.path
           FROM %T pkg JOIN %T p ON p.packageID = pkg.id
-          WHERE p.path IN (%Ls) AND pkg.status IN (%Ls) %Q',
+          WHERE p.pathIndex IN (%Ls) AND pkg.status IN (%Ls) %Q',
         $package->getTableName(),
         $path->getTableName(),
-        $chunk,
+        $indexes,
         array(
           self::STATUS_ACTIVE,
         ),
@@ -581,6 +599,18 @@ final class PhabricatorOwnersPackage
         ->setKey('owners')
         ->setType('list<map<string, wild>>')
         ->setDescription(pht('List of package owners.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('review')
+        ->setType('map<string, wild>')
+        ->setDescription(pht('Auto review information.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('audit')
+        ->setType('map<string, wild>')
+        ->setDescription(pht('Auto audit information.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('dominion')
+        ->setType('map<string, wild>')
+        ->setDescription(pht('Dominion setting information.')),
     );
   }
 
@@ -592,11 +622,56 @@ final class PhabricatorOwnersPackage
       );
     }
 
+    $review_map = self::getAutoreviewOptionsMap();
+    $review_value = $this->getAutoReview();
+    if (isset($review_map[$review_value])) {
+      $review_label = $review_map[$review_value]['name'];
+    } else {
+      $review_label = pht('Unknown ("%s")', $review_value);
+    }
+
+    $review = array(
+      'value' => $review_value,
+      'label' => $review_label,
+    );
+
+    if ($this->getAuditingEnabled()) {
+      $audit_value = 'audit';
+      $audit_label = pht('Auditing Enabled');
+    } else {
+      $audit_value = 'none';
+      $audit_label = pht('No Auditing');
+    }
+
+    $audit = array(
+      'value' => $audit_value,
+      'label' => $audit_label,
+    );
+
+    $dominion_value = $this->getDominion();
+    $dominion_map = self::getDominionOptionsMap();
+    if (isset($dominion_map[$dominion_value])) {
+      $dominion_label = $dominion_map[$dominion_value]['name'];
+      $dominion_short = $dominion_map[$dominion_value]['short'];
+    } else {
+      $dominion_label = pht('Unknown ("%s")', $dominion_value);
+      $dominion_short = pht('Unknown ("%s")', $dominion_value);
+    }
+
+    $dominion = array(
+      'value' => $dominion_value,
+      'label' => $dominion_label,
+      'short' => $dominion_short,
+    );
+
     return array(
       'name' => $this->getName(),
       'description' => $this->getDescription(),
       'status' => $this->getStatus(),
       'owners' => $owner_list,
+      'review' => $review,
+      'audit' => $audit,
+      'dominion' => $dominion,
     );
   }
 

src/applications/owners/storage/PhabricatorOwnersPath.php

@@ -4,7 +4,9 @@ final class PhabricatorOwnersPath extends PhabricatorOwnersDAO {
 
   protected $packageID;
   protected $repositoryPHID;
+  protected $pathIndex;
   protected $path;
+  protected $pathDisplay;
   protected $excluded;
 
   private $fragments;
@@ -14,23 +16,35 @@ final class PhabricatorOwnersPath extends PhabricatorOwnersDAO {
     return array(
       self::CONFIG_TIMESTAMPS => false,
       self::CONFIG_COLUMN_SCHEMA => array(
-        'path' => 'text255',
+        'path' => 'text',
+        'pathDisplay' => 'text',
+        'pathIndex' => 'bytes12',
         'excluded' => 'bool',
       ),
       self::CONFIG_KEY_SCHEMA => array(
-        'packageID' => array(
+        'key_path' => array(
-          'columns' => array('packageID'),
+          'columns' => array('packageID', 'repositoryPHID', 'pathIndex'),
+          'unique' => true,
+        ),
+        'key_repository' => array(
+          'columns' => array('repositoryPHID', 'pathIndex'),
         ),
       ),
     ) + parent::getConfiguration();
   }
 
-
   public static function newFromRef(array $ref) {
     $path = new PhabricatorOwnersPath();
     $path->repositoryPHID = $ref['repositoryPHID'];
-    $path->path = $ref['path'];
+
+    $raw_path = $ref['path'];
+
+    $path->pathIndex = PhabricatorHash::digestForIndex($raw_path);
+    $path->path = $raw_path;
+    $path->pathDisplay = $raw_path;
+
     $path->excluded = $ref['excluded'];
+
     return $path;
   }
 
@@ -38,6 +52,7 @@ final class PhabricatorOwnersPath extends PhabricatorOwnersDAO {
     return array(
       'repositoryPHID' => $this->getRepositoryPHID(),
       'path' => $this->getPath(),
+      'display' => $this->getPathDisplay(),
       'excluded' => (int)$this->getExcluded(),
     );
   }

src/applications/owners/xaction/PhabricatorOwnersPackagePathsTransaction.php

@@ -103,6 +103,26 @@ final class PhabricatorOwnersPackagePathsTransaction
 
     $paths = $object->getPaths();
 
+    // We store paths in a normalized format with a trailing slash, regardless
+    // of whether the user enters "path/to/file.c" or "src/backend/". Normalize
+    // paths now.
+
+    $display_map = array();
+    foreach ($new as $key => $spec) {
+      $display_path = $spec['path'];
+      $raw_path = rtrim($display_path, '/').'/';
+
+      // If the user entered two paths which normalize to the same value
+      // (like "src/main.c" and "src/main.c/"), discard the duplicates.
+      if (isset($display_map[$raw_path])) {
+        unset($new[$key]);
+        continue;
+      }
+
+      $new[$key]['path'] = $raw_path;
+      $display_map[$raw_path] = $display_path;
+    }
+
     $diffs = PhabricatorOwnersPath::getTransactionValueChanges($old, $new);
     list($rem, $add) = $diffs;
 
@@ -111,12 +131,24 @@ final class PhabricatorOwnersPackagePathsTransaction
       $ref = $path->getRef();
       if (PhabricatorOwnersPath::isRefInSet($ref, $set)) {
         $path->delete();
+        continue;
+      }
+
+      // If the user has changed the display value for a path but the raw
+      // storage value hasn't changed, update the display value.
+
+      if (isset($display_map[$path->getPath()])) {
+        $path
+          ->setPathDisplay($display_map[$path->getPath()])
+          ->save();
+        continue;
       }
     }
 
     foreach ($add as $ref) {
       $path = PhabricatorOwnersPath::newFromRef($ref)
         ->setPackageID($object->getID())
+        ->setPathDisplay($display_map[$ref['path']])
         ->save();
     }
   }

src/applications/phriction/storage/PhrictionTransaction.php

@@ -54,18 +54,6 @@ final class PhrictionTransaction
     return parent::shouldHideForMail($xactions);
   }
 
-  public function shouldHideForFeed() {
-    switch ($this->getTransactionType()) {
-      case PhrictionDocumentMoveToTransaction::TRANSACTIONTYPE:
-      case PhrictionDocumentMoveAwayTransaction::TRANSACTIONTYPE:
-        return true;
-      case PhrictionDocumentTitleTransaction::TRANSACTIONTYPE:
-        return $this->getMetadataValue('stub:create:phid', false);
-    }
-    return parent::shouldHideForFeed();
-  }
-
-
   public function getMailTags() {
     $tags = array();
     switch ($this->getTransactionType()) {

src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php

@@ -59,4 +59,8 @@ final class PhrictionDocumentMoveAwayTransaction
     return 'fa-arrows';
   }
 
+  public function shouldHideForFeed() {
+    return true;
+  }
+
 }

src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php

@@ -102,4 +102,8 @@ final class PhrictionDocumentMoveToTransaction
     return 'fa-arrows';
   }
 
+  public function shouldHideForFeed() {
+    return true;
+  }
+
 }

src/applications/releeph/field/specification/ReleephDiffChurnFieldSpecification.php

@@ -37,7 +37,7 @@ final class ReleephDiffChurnFieldSpecification
         case PhabricatorTransactions::TYPE_COMMENT:
           $comments++;
           break;
-        case DifferentialTransaction::TYPE_UPDATE:
+        case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
           $updates++;
           break;
         case DifferentialTransaction::TYPE_ACTION:

src/applications/repository/engine/PhabricatorRepositoryEngine.php

@@ -56,19 +56,18 @@ abstract class PhabricatorRepositoryEngine extends Phobject {
     $lock_key,
     $lock_device_only) {
 
-    $lock_parts = array();
+    $lock_parts = array(
-    $lock_parts[] = $lock_key;
+      'repositoryPHID' => $repository->getPHID(),
-    $lock_parts[] = $repository->getID();
+    );
 
     if ($lock_device_only) {
       $device = AlmanacKeys::getLiveDevice();
       if ($device) {
-        $lock_parts[] = $device->getID();
+        $lock_parts['devicePHID'] = $device->getPHID();
       }
     }
 
-    $lock_name = implode(':', $lock_parts);
+    return PhabricatorGlobalLock::newLock($lock_key, $lock_parts);
-    return PhabricatorGlobalLock::newLock($lock_name);
   }
 
 

src/applications/repository/query/PhabricatorRepositorySearchEngine.php

@@ -24,6 +24,9 @@ final class PhabricatorRepositorySearchEngine
       id(new PhabricatorSearchStringListField())
         ->setLabel(pht('Callsigns'))
         ->setKey('callsigns'),
+      id(new PhabricatorSearchStringListField())
+        ->setLabel(pht('Short Names'))
+        ->setKey('shortNames'),
       id(new PhabricatorSearchSelectField())
         ->setLabel(pht('Status'))
         ->setKey('status')
@@ -51,6 +54,10 @@ final class PhabricatorRepositorySearchEngine
       $query->withCallsigns($map['callsigns']);
     }
 
+    if ($map['shortNames']) {
+      $query->withSlugs($map['shortNames']);
+    }
+
     if ($map['status']) {
       $status = idx($this->getStatusValues(), $map['status']);
       if ($status) {

src/applications/transactions/editengine/PhabricatorEditEngine.php

@@ -2092,7 +2092,7 @@ abstract class PhabricatorEditEngine
 
     return array(
       'object' => array(
-        'id' => $object->getID(),
+        'id' => (int)$object->getID(),
         'phid' => $object->getPHID(),
       ),
       'transactions' => $xactions_struct,

src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php

@@ -6,6 +6,8 @@
 class PhabricatorApplicationTransactionFeedStory
   extends PhabricatorFeedStory {
 
+  private $primaryTransactionPHID;
+
   public function getPrimaryObjectPHID() {
     return $this->getValue('objectPHID');
   }
@@ -27,7 +29,36 @@ class PhabricatorApplicationTransactionFeedStory
   }
 
   protected function getPrimaryTransactionPHID() {
-    return head($this->getValue('transactionPHIDs'));
+    if ($this->primaryTransactionPHID === null) {
+      // Transactions are filtered and sorted before they're stored, but the
+      // rendering logic can change between the time an edit occurs and when
+      // we actually render the story. Recalculate the filtering at display
+      // time because it's cheap and gets us better results when things change
+      // by letting the changes apply retroactively.
+
+      $xaction_phids = $this->getValue('transactionPHIDs');
+
+      $xactions = array();
+      foreach ($xaction_phids as $xaction_phid) {
+        $xactions[] = $this->getObject($xaction_phid);
+      }
+
+      foreach ($xactions as $key => $xaction) {
+        if ($xaction->shouldHideForFeed()) {
+          unset($xactions[$key]);
+        }
+      }
+
+      if ($xactions) {
+        $primary_phid = head($xactions)->getPHID();
+      } else {
+        $primary_phid = head($xaction_phids);
+      }
+
+      $this->primaryTransactionPHID = $primary_phid;
+    }
+
+    return $this->primaryTransactionPHID;
   }
 
   public function getPrimaryTransaction() {

src/applications/transactions/storage/PhabricatorModularTransaction.php

@@ -92,6 +92,14 @@ abstract class PhabricatorModularTransaction
     return parent::shouldHide();
   }
 
+  final public function shouldHideForFeed() {
+    if ($this->getTransactionImplementation()->shouldHideForFeed()) {
+      return true;
+    }
+
+    return parent::shouldHideForFeed();
+  }
+
   /* final */ public function getIcon() {
     $icon = $this->getTransactionImplementation()->getIcon();
     if ($icon !== null) {

src/applications/transactions/storage/PhabricatorModularTransactionType.php

@@ -47,6 +47,10 @@ abstract class PhabricatorModularTransactionType
     return false;
   }
 
+  public function shouldHideForFeed() {
+    return false;
+  }
+
   public function getIcon() {
     return null;
   }

src/docs/user/userguide/owners.diviner

@@ -84,21 +84,26 @@ You can configure **Auto Review** for packages. When a new code review is
 created in Differential which affects code in a package, the package can
 automatically be added as a subscriber or reviewer.
 
-The available settings are:
+The available settings allow you to take these actions:
 
-  - **No Autoreview**: This package will not be added to new reviews.
-  - **Subscribe to Changes**: This package will be added to reviews as a
-    subscriber. Owners will be notified of changes, but not required to act.
   - **Review Changes**: This package will be added to reviews as a reviewer.
     Reviews will appear on the dashboards of package owners.
-  - **Review Changes (Blocking)** This package will be added to reviews
+  - **Review Changes (Blocking)** This package will be added to reviews as a
-    as a blocking reviewer. A package owner will be required to accept changes
+    blocking reviewer. A package owner will be required to accept changes
     before they may land.
+  - **Subscribe to Changes**: This package will be added to reviews as a
+    subscriber. Owners will be notified of changes, but not required to act.
 
-NOTE: These rules **do not trigger** if the change author is a package owner.
+If you select the **With Non-Owner Author** option for these actions, the
-They only apply to changes made by users who aren't already owners.
+action will not trigger if the author of the revision is a package owner. This
+mode may be helpful if you are using Owners mostly to make sure that someone
+who is qualified is involved in each change to a piece of code.
 
-These rules also do not trigger if the package has been archived.
+If you select the **All** option for these actions, the action will always
+trigger even if the author is a package owner. This mode may be helpful if you
+are using Owners mostly to suggest reviewers.
+
+These rules do not trigger if the package has been archived.
 
 The intent of this feature is to make it easy to configure simple, reasonable
 behaviors. If you want more tailored or specific triggers, you can write more

src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollector.php

@@ -100,8 +100,10 @@ abstract class PhabricatorGarbageCollector extends Phobject {
 
     // Hold a lock while performing collection to avoid racing other daemons
     // running the same collectors.
-    $lock_name = 'gc:'.$this->getCollectorConstant();
+    $params = array(
-    $lock = PhabricatorGlobalLock::newLock($lock_name);
+      'collector' => $this->getCollectorConstant(),
+    );
+    $lock = PhabricatorGlobalLock::newLock('gc', $params);
 
     try {
       $lock->lock(5);

src/infrastructure/markup/view/PHUIRemarkupImageView.php

@@ -0,0 +1,79 @@
+<?php
+
+final class PHUIRemarkupImageView
+  extends AphrontView {
+
+  private $uri;
+  private $width;
+  private $height;
+  private $alt;
+  private $classes = array();
+
+  public function setURI($uri) {
+    $this->uri = $uri;
+    return $this;
+  }
+
+  public function getURI() {
+    return $this->uri;
+  }
+
+  public function setWidth($width) {
+    $this->width = $width;
+    return $this;
+  }
+
+  public function getWidth() {
+    return $this->width;
+  }
+
+  public function setHeight($height) {
+    $this->height = $height;
+    return $this;
+  }
+
+  public function getHeight() {
+    return $this->height;
+  }
+
+  public function setAlt($alt) {
+    $this->alt = $alt;
+    return $this;
+  }
+
+  public function getAlt() {
+    return $this->alt;
+  }
+
+  public function addClass($class) {
+    $this->classes[] = $class;
+    return $this;
+  }
+
+  public function render() {
+    $id = celerity_generate_unique_node_id();
+
+    Javelin::initBehavior(
+      'remarkup-load-image',
+      array(
+        'uri' => (string)$this->uri,
+        'imageID' => $id,
+      ));
+
+    $classes = null;
+    if ($this->classes) {
+      $classes = implode(' ', $this->classes);
+    }
+
+    return phutil_tag(
+      'img',
+      array(
+        'id' => $id,
+        'width' => $this->getWidth(),
+        'height' => $this->getHeight(),
+        'alt' => $this->getAlt(),
+        'class' => $classes,
+      ));
+  }
+
+}

src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php

@@ -1163,12 +1163,16 @@ abstract class PhabricatorStorageManagementWorkflow
     // Although we're holding this lock on different databases so it could
     // have the same name on each as far as the database is concerned, the
     // locks would be the same within this process.
-    $ref_key = $api->getRef()->getRefKey();
+    $parameters = array(
-    $ref_hash = PhabricatorHash::digestForIndex($ref_key);
+      'refKey' => $api->getRef()->getRefKey(),
-    $lock_name = 'adjust('.$ref_hash.')';
+    );
 
-    return PhabricatorGlobalLock::newLock($lock_name)
+    // We disable logging for this lock because we may not have created the
+    // log table yet, or may need to adjust it.
+
+    return PhabricatorGlobalLock::newLock('adjust', $parameters)
       ->useSpecificConnection($api->getConn(null))
+      ->setDisableLogging(true)
       ->lock();
   }
 

src/infrastructure/util/PhabricatorGlobalLock.php

@@ -28,8 +28,11 @@
  */
 final class PhabricatorGlobalLock extends PhutilLock {
 
+  private $parameters;
   private $conn;
   private $isExternalConnection = false;
+  private $log;
+  private $disableLogging;
 
   private static $pool = array();
 
@@ -37,27 +40,42 @@ final class PhabricatorGlobalLock extends PhutilLock {
 /* -(  Constructing Locks  )------------------------------------------------- */
 
 
-  public static function newLock($name) {
+  public static function newLock($name, $parameters = array()) {
     $namespace = PhabricatorLiskDAO::getStorageNamespace();
     $namespace = PhabricatorHash::digestToLength($namespace, 20);
 
-    $full_name = 'ph:'.$namespace.':'.$name;
+    $parts = array();
-
+    ksort($parameters);
-    $length_limit = 64;
+    foreach ($parameters as $key => $parameter) {
-    if (strlen($full_name) > $length_limit) {
+      if (!preg_match('/^[a-zA-Z0-9]+\z/', $key)) {
         throw new Exception(
           pht(
-          'Lock name "%s" is too long (full lock name is "%s"). The '.
+            'Lock parameter key "%s" must be alphanumeric.',
-          'full lock name must not be longer than %s bytes.',
+            $key));
-          $name,
-          $full_name,
-          new PhutilNumber($length_limit)));
       }
 
+      if (!is_scalar($parameter) && !is_null($parameter)) {
+        throw new Exception(
+          pht(
+            'Lock parameter for key "%s" must be a scalar.',
+            $key));
+      }
+
+      $value = phutil_json_encode($parameter);
+      $parts[] = "{$key}={$value}";
+    }
+    $parts = implode(', ', $parts);
+
+    $local = "{$name}({$parts})";
+    $local = PhabricatorHash::digestToLength($local, 20);
+
+    $full_name = "ph:{$namespace}:{$local}";
     $lock = self::getLock($full_name);
     if (!$lock) {
       $lock = new PhabricatorGlobalLock($full_name);
       self::registerLock($lock);
+
+      $lock->parameters = $parameters;
     }
 
     return $lock;
@@ -79,6 +97,11 @@ final class PhabricatorGlobalLock extends PhutilLock {
     return $this;
   }
 
+  public function setDisableLogging($disable) {
+    $this->disableLogging = $disable;
+    return $this;
+  }
+
 
 /* -(  Implementation  )----------------------------------------------------- */
 
@@ -127,6 +150,24 @@ final class PhabricatorGlobalLock extends PhutilLock {
     $conn->rememberLock($lock_name);
 
     $this->conn = $conn;
+
+    if ($this->shouldLogLock()) {
+      global $argv;
+
+      $lock_context = array(
+        'pid' => getmypid(),
+        'host' => php_uname('n'),
+        'argv' => $argv,
+      );
+
+      $log = id(new PhabricatorDaemonLockLog())
+        ->setLockName($lock_name)
+        ->setLockParameters($this->parameters)
+        ->setLockContext($lock_context)
+        ->save();
+
+      $this->log = $log;
+    }
   }
 
   protected function doUnlock() {
@@ -159,6 +200,32 @@ final class PhabricatorGlobalLock extends PhutilLock {
       $conn->close();
       self::$pool[] = $conn;
     }
+
+    if ($this->log) {
+      $log = $this->log;
+      $this->log = null;
+
+      $conn = $log->establishConnection('w');
+      queryfx(
+        $conn,
+        'UPDATE %T SET lockReleased = UNIX_TIMESTAMP() WHERE id = %d',
+        $log->getTableName(),
+        $log->getID());
+    }
+  }
+
+  private function shouldLogLock() {
+    if ($this->disableLogging) {
+      return false;
+    }
+
+    $policy = id(new PhabricatorDaemonLockLogGarbageCollector())
+      ->getRetentionPolicy();
+    if (!$policy) {
+      return false;
+    }
+
+    return true;
   }
 
 }

webroot/rsrc/css/application/owners/owners-path-editor.css

@@ -3,7 +3,8 @@
  */
 
 .owners-path-editor-table {
-  margin: 10px;
+  margin: 10px 0;
+  width: 100%;
 }
 
 .owners-path-editor-table td {
@@ -11,27 +12,38 @@
   vertical-align: middle;
 }
 
-.owners-path-editor-table select.owners-repo {
+.owners-path-editor-table td.owners-path-mode-control {
-  width: 150px;
+  width: 180px;
 }
 
-.owners-path-editor-table input {
+.owners-path-editor-table td.owners-path-mode-control select {
-  width: 400px;
+  width: 100%;
 }
 
-.owners-path-editor-table div.error-display {
+.owners-path-editor-table td.owners-path-repo-control {
-  padding: 4px 12px 0;
+  width: 280px;
 }
 
-.owners-path-editor-table div.validating {
+.owners-path-editor-table td.owners-path-path-control {
-  color: {$greytext};
+  width: auto;
 }
 
-.owners-path-editor-table div.invalid {
+.owners-path-editor-table td.owners-path-path-control input {
-  color: #aa0000;
+  width: 100%;
 }
 
-.owners-path-editor-table div.valid {
+.owners-path-editor-table td.owners-path-path-control .jx-typeahead-results a {
-  color: #00aa00;
+  padding: 4px;
-  font-weight: bold;
+}
+
+.owners-path-editor-table td.owners-path-icon-control {
+  width: 18px;
+}
+
+.owners-path-editor-table td.remove-column {
+  width: 100px;
+}
+
+.owners-path-editor-table td.remove-column a {
+  display: block;
 }

webroot/rsrc/css/core/remarkup.css

@@ -472,6 +472,13 @@ video.phabricator-media {
   margin: .5em 1em 0;
 }
 
+.phabricator-remarkup-image-error {
+  border: 1px solid {$redborder};
+  background: {$sh-redbackground};
+  padding: 8px 12px;
+  color: {$darkgreytext};
+}
+
 .phabricator-remarkup-embed-image {
   display: inline-block;
   border: 3px solid white;

webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js

@@ -9,8 +9,7 @@
 
 JX.install('TypeaheadSource', {
   construct : function() {
-    this._raw = {};
+    this.resetResults();
-    this._lookup = {};
     this.setNormalizer(JX.TypeaheadNormalizer.normalize);
     this._excludeIDs = {};
   },
@@ -359,6 +358,12 @@ JX.install('TypeaheadSource', {
       }
       return str.split(/\s+/g);
     },
+
+    resetResults: function() {
+      this._raw = {};
+      this._lookup = {};
+    },
+
     _defaultTransformer : function(object) {
       return {
         name : object[0],

webroot/rsrc/js/application/herald/PathTypeahead.js

@@ -11,27 +11,22 @@
 
 JX.install('PathTypeahead', {
   construct : function(config) {
-    this._repositorySelect = config.repo_select;
+    this._repositoryTokenizer = config.repositoryTokenizer;
     this._hardpoint = config.hardpoint;
     this._input = config.path_input;
     this._completeURI = config.completeURI;
     this._validateURI = config.validateURI;
     this._errorDisplay = config.error_display;
+    this._textInputValues = {};
 
-    /*
+    this._icons = config.icons;
-     * Default values to preload the typeahead with, for extremely common
-     * cases.
-     */
-    this._textInputValues = config.repositoryDefaultPaths;
 
     this._initializeDatasource();
     this._initializeTypeahead(this._input);
   },
   members : {
-    /*
+    _repositoryTokenizer : null,
-     * DOM <select> elem for choosing the repository of a path.
+
-     */
-    _repositorySelect : null,
     /*
      * DOM parent div "hardpoint" to be passed to the JX.Typeahead.
      */
@@ -84,31 +79,28 @@ JX.install('PathTypeahead', {
      */
     start : function() {
       if (this._typeahead.getValue()) {
-        this._textInputValues[this._repositorySelect.value] =
+        var phid = this._getRepositoryPHID();
-          this._typeahead.getValue();
+        if (phid) {
+          this._textInputValues[phid] = this._typeahead.getValue();
+        }
       }
 
       this._typeahead.listen(
         'change',
         JX.bind(this, function(value) {
-          this._textInputValues[this._repositorySelect.value] = value;
+          var phid = this._getRepositoryPHID();
-          this._validate();
+          if (phid) {
-        }));
+            this._textInputValues[phid] = value;
+          }
 
-      this._typeahead.listen(
+          this._validate();
-        'choose',
-        JX.bind(this, function() {
-          setTimeout(JX.bind(this._typeahead, this._typeahead.refresh), 0);
         }));
 
       var repo_set_input = JX.bind(this, this._onrepochange);
 
       this._typeahead.listen('start', repo_set_input);
-      JX.DOM.listen(
+
-        this._repositorySelect,
+      this._repositoryTokenizer.listen('change', repo_set_input);
-        'change',
-        null,
-        repo_set_input);
 
       this._typeahead.start();
       this._validate();
@@ -120,13 +112,18 @@ JX.install('PathTypeahead', {
         this._textInputValues);
 
       this._datasource.setAuxiliaryData(
-        {repositoryPHID : this._repositorySelect.value}
+        {
-      );
+          repositoryPHID: this._getRepositoryPHID()
+        });
+
+      // Since we've changed the repository, reset the results.
+      this._datasource.resetResults();
     },
 
     _setPathInputBasedOnRepository : function(typeahead, lookup) {
-      if (lookup[this._repositorySelect.value]) {
+      var phid = this._getRepositoryPHID();
-        typeahead.setValue(lookup[this._repositorySelect.value]);
+      if (phid && lookup[phid]) {
+        typeahead.setValue(lookup[phid]);
       } else {
         typeahead.setValue('/');
       }
@@ -152,9 +149,24 @@ JX.install('PathTypeahead', {
       return ('' + str).replace(/[\/]+/g, '\/');
     },
 
+    _getRepositoryPHID: function() {
+      var tokens = this._repositoryTokenizer.getTokens();
+      var keys = JX.keys(tokens);
+
+      if (keys.length) {
+        return keys[0];
+      }
+
+      return null;
+    },
+
     _validate : function() {
+      var repo_phid = this._getRepositoryPHID();
+      if (!repo_phid) {
+        return;
+      }
+
       var input = this._input;
-      var repo_id = this._repositorySelect.value;
       var input_value = input.value;
       var error_display = this._errorDisplay;
 
@@ -170,33 +182,32 @@ JX.install('PathTypeahead', {
 
       var validation_request = new JX.Request(
         this._validateURI,
-        function(payload) {
+        JX.bind(this, function(payload) {
           // Don't change validation display state if the input has been
           // changed since we started validation
-          if (input.value === input_value) {
+          if (input.value !== input_value) {
+            return;
+          }
+
           if (payload.valid) {
-              JX.DOM.alterClass(error_display, 'invalid', false);
+            JX.DOM.setContent(error_display, JX.$H(this._icons.okay));
-              JX.DOM.alterClass(error_display, 'valid', true);
           } else {
-              JX.DOM.alterClass(error_display, 'invalid', true);
+            JX.DOM.setContent(error_display, JX.$H(this._icons.fail));
-              JX.DOM.alterClass(error_display, 'valid', false);
           }
-            JX.DOM.setContent(error_display, payload.message);
+        }));
-          }
-        });
 
       validation_request.listen('finally', function() {
-        JX.DOM.alterClass(error_display, 'validating', false);
         this._validationInflight = null;
       });
 
       validation_request.setData(
         {
-          repositoryPHID : repo_id,
+          repositoryPHID : repo_phid,
           path : input_value
         });
 
       this._validationInflight = validation_request;
+      JX.DOM.setContent(error_display, JX.$H(this._icons.test));
 
       validation_request.setTimeout(750);
       validation_request.send();

webroot/rsrc/js/application/owners/OwnersPathEditor.js

@@ -5,6 +5,7 @@
  *           javelin-dom
  *           javelin-util
  *           phabricator-prefab
+ *           phuix-form-control-view
  * @provides owners-path-editor
  * @javelin
  */
@@ -23,12 +24,13 @@ JX.install('OwnersPathEditor', {
       JX.bind(this, this._onaddpath));
 
     this._count = 0;
-    this._repositories = config.repositories;
     this._inputTemplate = config.input_template;
+    this._repositoryTokenizerSpec = config.repositoryTokenizerSpec;
 
     this._completeURI = config.completeURI;
     this._validateURI = config.validateURI;
-    this._repositoryDefaultPaths = config.repositoryDefaultPaths;
+    this._icons = config.icons;
+    this._modeOptions = config.modeOptions;
 
     this._initializePaths(config.pathRefs);
   },
@@ -38,12 +40,6 @@ JX.install('OwnersPathEditor', {
      */
     _rowManager : null,
 
-    /*
-     * Array of objects with 'name' and 'repo_id' keys for
-     * selecting the repository of a path.
-     */
-    _repositories : null,
-
     /*
      * How many rows have been created, for form name generation.
      */
@@ -66,8 +62,8 @@ JX.install('OwnersPathEditor', {
      * default for future rows.
      */
     _lastRepositoryChoice : null,
-
+    _icons: null,
-    _repositoryDefaultPaths : null,
+    _modeOptions: null,
 
     /*
      * Initialize with 0 or more rows.
@@ -88,69 +84,35 @@ JX.install('OwnersPathEditor', {
     addPath : function(path_ref) {
       // Smart default repository. See _lastRepositoryChoice.
       if (path_ref) {
-        this._lastRepositoryChoice = path_ref.repositoryPHID;
+        this._lastRepositoryChoice = path_ref.repositoryValue;
-      }
+      } else {
-      path_ref = path_ref || {};
+        path_ref = {
-
+          repositoryValue: this._lastRepositoryChoice || {}
-      var selected_repository = path_ref.repositoryPHID ||
-        this._lastRepositoryChoice;
-      var options = this._buildRepositoryOptions(selected_repository);
-      var attrs = {
-        name : 'repo[' + this._count + ']',
-        className : 'owners-repo'
         };
-      var repo_select = JX.$N('select', attrs, options);
+      }
 
-      JX.DOM.listen(repo_select, 'change', null, JX.bind(this, function() {
+      var repo = this._newRepoCell(path_ref.repositoryValue);
-        this._lastRepositoryChoice = repo_select.value;
+      var path = this._newPathCell(path_ref.display);
-      }));
+      var icon = this._newIconCell();
-
+      var mode_cell = this._newModeCell(path_ref.excluded);
-      var repo_cell = JX.$N('td', {}, repo_select);
-      var typeahead_cell = JX.$N(
-        'td',
-        JX.$H(this._inputTemplate));
-
-      // Text input for path.
-      var path_input = JX.DOM.find(typeahead_cell, 'input');
-      JX.copy(
-        path_input,
-        {
-          value : path_ref.path || '',
-          name : 'path[' + this._count + ']'
-        });
-
-      // The Typeahead requires a display div called hardpoint.
-      var hardpoint = JX.DOM.find(
-        typeahead_cell,
-        'div',
-        'typeahead-hardpoint');
-
-      var error_display = JX.$N(
-        'div',
-        {
-          className : 'error-display validating'
-        },
-        'Validating...');
-
-      var error_display_cell = JX.$N('td', {}, error_display);
-
-      var exclude = JX.Prefab.renderSelect(
-        {'0' : 'Include', '1' : 'Exclude'},
-        path_ref.excluded,
-        {name : 'exclude[' + this._count + ']'});
-      var exclude_cell = JX.$N('td', {}, exclude);
 
       var row = this._rowManager.addRow(
-        [exclude_cell, repo_cell, typeahead_cell, error_display_cell]);
+        [
+          mode_cell,
+          repo.cell,
+          path.cell,
+          icon.cell
+        ]);
 
       new JX.PathTypeahead({
-        repositoryDefaultPaths : this._repositoryDefaultPaths,
+        repositoryTokenizer: repo.tokenizer,
-        repo_select : repo_select,
+        path_input : path.input,
-        path_input : path_input,
+        hardpoint : path.hardpoint,
-        hardpoint : hardpoint,
+        error_display : icon.cell,
-        error_display : error_display,
         completeURI : this._completeURI,
-        validateURI : this._validateURI}).start();
+        validateURI : this._validateURI,
+        icons: this._icons
+      }).start();
 
       this._count++;
       return row;
@@ -161,20 +123,109 @@ JX.install('OwnersPathEditor', {
       this.addPath();
     },
 
-    /**
+    _newModeCell: function(value) {
-     * Helper to build the options for the repository choice dropdown.
+      var options = this._modeOptions;
-     */
+
-    _buildRepositoryOptions : function(selected) {
+      var name = 'exclude[' + this._count + ']';
-      var repos = this._repositories;
+
-      var result = [];
+      var control = JX.Prefab.renderSelect(options, value, {name: name});
-      for (var k in repos) {
+
-        var attr = {
+      return JX.$N(
-          value : k,
+        'td',
-          selected : (selected == k)
+        {
+          className: 'owners-path-mode-control'
+        },
+        control);
+    },
+
+    _newRepoCell: function(value) {
+      var repo_control = new JX.PHUIXFormControl()
+        .setControl('tokenizer', this._repositoryTokenizerSpec)
+        .setValue(value);
+
+      var repo_tokenizer = repo_control.getTokenizer();
+      var name = 'repo[' + this._count + ']';
+
+      function get_phid() {
+        var phids = repo_control.getValue();
+        if (!phids.length) {
+          return null;
+        }
+
+        return phids[0];
+      }
+
+      var input = JX.$N(
+        'input',
+        {
+          type: 'hidden',
+          name: name,
+          value: get_phid()
+        });
+
+      repo_tokenizer.listen('change', JX.bind(this, function() {
+        this._lastRepositoryChoice = repo_tokenizer.getTokens();
+
+        input.value = get_phid();
+      }));
+
+      var cell = JX.$N(
+        'td',
+        {
+          className: 'owners-path-repo-control'
+        },
+        [
+          repo_control.getRawInputNode(),
+          input
+        ]);
+
+      return {
+        cell: cell,
+        tokenizer: repo_tokenizer
+      };
+    },
+
+    _newPathCell: function(value) {
+      var path_cell = JX.$N(
+        'td',
+        {
+          className: 'owners-path-path-control'
+        },
+        JX.$H(this._inputTemplate));
+
+      var path_input = JX.DOM.find(path_cell, 'input');
+
+      JX.copy(
+        path_input,
+        {
+          value: value || '',
+          name: 'path[' + this._count + ']'
+        });
+
+      var hardpoint = JX.DOM.find(
+        path_cell,
+        'div',
+        'typeahead-hardpoint');
+
+      return {
+        cell: path_cell,
+        input: path_input,
+        hardpoint: hardpoint
+      };
+    },
+
+    _newIconCell: function() {
+      var cell = JX.$N(
+        'td',
+        {
+          className: 'owners-path-icon-control'
+        });
+
+      return {
+        cell: cell
       };
-        result.push(JX.$N('option', attr, repos[k]));
-      }
-      return result;
     }
+
   }
+
 });

webroot/rsrc/js/core/behavior-lightbox-attachments.js

@@ -56,6 +56,10 @@ JX.behavior('lightbox-attachments', function() {
 
     e.kill();
 
+    activateLightbox(e.getNode('lightboxable'));
+  }
+
+  function activateLightbox(target) {
     var mainFrame = JX.$('main-page-frame');
     var links = JX.DOM.scry(mainFrame, '*', 'lightboxable');
     var phids = {};
@@ -68,7 +72,6 @@ JX.behavior('lightbox-attachments', function() {
     // Now that we have the big picture phid situation sorted out, figure
     // out how the actual node the user clicks fits into that big picture
     // and build some pretty UI to show the attachment.
-    var target      = e.getNode('lightboxable');
     var target_data = JX.Stratcom.getData(target);
     var total       = JX.keys(phids).length;
     var current     = 1;
@@ -324,7 +327,8 @@ JX.behavior('lightbox-attachments', function() {
     }
     e.prevent();
     closeLightBox(e);
-    el.click();
+
+    activateLightbox(el);
   }
 
   // Only look for lightboxable inside the main page, not other lightboxes.

webroot/rsrc/js/core/behavior-remarkup-load-image.js

@@ -0,0 +1,45 @@
+/**
+ * @provides javelin-behavior-remarkup-load-image
+ * @requires javelin-behavior
+ *           javelin-request
+ */
+
+JX.behavior('remarkup-load-image', function(config) {
+
+  function get_node() {
+    try {
+      return JX.$(config.imageID);
+    } catch (ex) {
+      return null;
+    }
+  }
+
+  function onload(r) {
+    var node = get_node();
+    if (!node) {
+      return;
+    }
+
+    node.src = r.imageURI;
+  }
+
+  function onerror(r) {
+    var node = get_node();
+    if (!node) {
+      return;
+    }
+
+    var error = JX.$N(
+      'div',
+      {
+        className: 'phabricator-remarkup-image-error'
+      },
+      r.info);
+
+    JX.DOM.replace(node, error);
+  }
+
+  var request = new JX.Request(config.uri, onload);
+  request.listen('error', onerror);
+  request.send();
+});

webroot/rsrc/js/phuix/PHUIXFormControl.js

@@ -15,6 +15,7 @@ JX.install('PHUIXFormControl', {
     _valueSetCallback: null,
     _valueGetCallback: null,
     _rawInputNode: null,
+    _tokenizer: null,
 
     setLabel: function(label) {
       JX.DOM.setContent(this._getLabelNode(), label);
@@ -70,6 +71,7 @@ JX.install('PHUIXFormControl', {
       this._valueGetCallback = input.get;
       this._valueSetCallback = input.set;
       this._rawInputNode = input.node;
+      this._tokenizer = input.tokenizer || null;
 
       return this;
     },
@@ -87,6 +89,10 @@ JX.install('PHUIXFormControl', {
       return this._rawInputNode;
     },
 
+    getTokenizer: function() {
+      return this._tokenizer;
+    },
+
     getNode: function() {
       if (!this._node) {
 
@@ -168,7 +174,8 @@ JX.install('PHUIXFormControl', {
       return {
         node: build.node,
         get: get_value,
-        set: set_value
+        set: set_value,
+        tokenizer: build.tokenizer
       };
     },