Load blame in Diffusion by AJAX

Summary:
I have blame enabled by default and displaying files with long history takes easily over 10 seconds.
Load the blame data by AJAX instead.
This is actually doing more work and the total response time is longer but it's worth it for me as I am interested just in the file contents quite often.

I know you were talking about building blame cache but until we have it...

I'm not sure if the AJAX loading indicator in bottom right corner is enough to inform the user that we are loading it on background.

Test Plan:
?view=highlighted
?view=plainblame
?view=blame

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

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

Author: vrana

scripts/celerity_mapper.php

@@ -136,6 +136,7 @@
     'javelin-behavior-aphront-drag-and-drop-textarea',
     'javelin-behavior-phabricator-object-selector',
     'javelin-behavior-repository-crossreference',
+    'javelin-behavior-load-blame',
 
     'differential-inline-comment-editor',
     'javelin-behavior-differential-dropdown-menus',

src/__celerity_resource_map__.php

@@ -928,7 +928,7 @@
   ),
   'diffusion-source-css' =>
   array(
-    'uri' => '/res/6a28b429/rsrc/css/application/diffusion/diffusion-source.css',
+    'uri' => '/res/e76bcd50/rsrc/css/application/diffusion/diffusion-source.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -1582,6 +1582,18 @@
     ),
     'disk' => '/rsrc/js/application/maniphest/behavior-line-chart.js',
   ),
+  'javelin-behavior-load-blame' =>
+  array(
+    'uri' => '/res/138e2961/rsrc/js/application/diffusion/behavior-load-blame.js',
+    'type' => 'js',
+    'requires' =>
+    array(
+      0 => 'javelin-behavior',
+      1 => 'javelin-dom',
+      2 => 'javelin-request',
+    ),
+    'disk' => '/rsrc/js/application/diffusion/behavior-load-blame.js',
+  ),
   'javelin-behavior-maniphest-batch-editor' =>
   array(
     'uri' => '/res/d22661be/rsrc/js/application/maniphest/behavior-batch-editor.js',
@@ -3601,7 +3613,7 @@
       'uri' => '/res/pkg/8aaacd1b/differential.pkg.css',
       'type' => 'css',
     ),
-    'd2447f72' =>
+    '322728f3' =>
     array(
       'name' => 'differential.pkg.js',
       'symbols' =>
@@ -3621,12 +3633,13 @@
         12 => 'javelin-behavior-aphront-drag-and-drop-textarea',
         13 => 'javelin-behavior-phabricator-object-selector',
         14 => 'javelin-behavior-repository-crossreference',
-        15 => 'differential-inline-comment-editor',
-        16 => 'javelin-behavior-differential-dropdown-menus',
-        17 => 'javelin-behavior-differential-toggle-files',
-        18 => 'javelin-behavior-differential-user-select',
+        15 => 'javelin-behavior-load-blame',
+        16 => 'differential-inline-comment-editor',
+        17 => 'javelin-behavior-differential-dropdown-menus',
+        18 => 'javelin-behavior-differential-toggle-files',
+        19 => 'javelin-behavior-differential-user-select',
       ),
-      'uri' => '/res/pkg/d2447f72/differential.pkg.js',
+      'uri' => '/res/pkg/322728f3/differential.pkg.js',
       'type' => 'js',
     ),
     'c8ce2d88' =>
@@ -3724,7 +3737,7 @@
     'aphront-typeahead-control-css' => '09aa1f68',
     'differential-changeset-view-css' => '8aaacd1b',
     'differential-core-view-css' => '8aaacd1b',
-    'differential-inline-comment-editor' => 'd2447f72',
+    'differential-inline-comment-editor' => '322728f3',
     'differential-local-commits-view-css' => '8aaacd1b',
     'differential-results-table-css' => '8aaacd1b',
     'differential-revision-add-comment-css' => '8aaacd1b',
@@ -3742,31 +3755,32 @@
     'javelin-behavior-aphlict-dropdown' => 'f24c209c',
     'javelin-behavior-aphlict-listen' => 'f24c209c',
     'javelin-behavior-aphront-basic-tokenizer' => 'f24c209c',
-    'javelin-behavior-aphront-drag-and-drop' => 'd2447f72',
-    'javelin-behavior-aphront-drag-and-drop-textarea' => 'd2447f72',
+    'javelin-behavior-aphront-drag-and-drop' => '322728f3',
+    'javelin-behavior-aphront-drag-and-drop-textarea' => '322728f3',
     'javelin-behavior-aphront-form-disable-on-submit' => 'f24c209c',
     'javelin-behavior-audit-preview' => 'f96657b8',
     'javelin-behavior-dark-console' => 'dca4a03d',
     'javelin-behavior-device' => 'f24c209c',
-    'javelin-behavior-differential-accept-with-errors' => 'd2447f72',
-    'javelin-behavior-differential-add-reviewers-and-ccs' => 'd2447f72',
-    'javelin-behavior-differential-comment-jump' => 'd2447f72',
-    'javelin-behavior-differential-diff-radios' => 'd2447f72',
-    'javelin-behavior-differential-dropdown-menus' => 'd2447f72',
-    'javelin-behavior-differential-edit-inline-comments' => 'd2447f72',
-    'javelin-behavior-differential-feedback-preview' => 'd2447f72',
-    'javelin-behavior-differential-keyboard-navigation' => 'd2447f72',
-    'javelin-behavior-differential-populate' => 'd2447f72',
-    'javelin-behavior-differential-show-more' => 'd2447f72',
-    'javelin-behavior-differential-toggle-files' => 'd2447f72',
-    'javelin-behavior-differential-user-select' => 'd2447f72',
+    'javelin-behavior-differential-accept-with-errors' => '322728f3',
+    'javelin-behavior-differential-add-reviewers-and-ccs' => '322728f3',
+    'javelin-behavior-differential-comment-jump' => '322728f3',
+    'javelin-behavior-differential-diff-radios' => '322728f3',
+    'javelin-behavior-differential-dropdown-menus' => '322728f3',
+    'javelin-behavior-differential-edit-inline-comments' => '322728f3',
+    'javelin-behavior-differential-feedback-preview' => '322728f3',
+    'javelin-behavior-differential-keyboard-navigation' => '322728f3',
+    'javelin-behavior-differential-populate' => '322728f3',
+    'javelin-behavior-differential-show-more' => '322728f3',
+    'javelin-behavior-differential-toggle-files' => '322728f3',
+    'javelin-behavior-differential-user-select' => '322728f3',
     'javelin-behavior-diffusion-commit-graph' => 'f96657b8',
     'javelin-behavior-diffusion-pull-lastmodified' => 'f96657b8',
     'javelin-behavior-error-log' => 'dca4a03d',
     'javelin-behavior-global-drag-and-drop' => 'f24c209c',
     'javelin-behavior-history-install' => 'f24c209c',
     'javelin-behavior-konami' => 'f24c209c',
     'javelin-behavior-lightbox-attachments' => 'f24c209c',
+    'javelin-behavior-load-blame' => '322728f3',
     'javelin-behavior-maniphest-batch-selector' => '7707de41',
     'javelin-behavior-maniphest-subpriority-editor' => '7707de41',
     'javelin-behavior-maniphest-transaction-controls' => '7707de41',
@@ -3776,15 +3790,15 @@
     'javelin-behavior-phabricator-autofocus' => 'f24c209c',
     'javelin-behavior-phabricator-keyboard-shortcuts' => 'f24c209c',
     'javelin-behavior-phabricator-nav' => 'f24c209c',
-    'javelin-behavior-phabricator-object-selector' => 'd2447f72',
+    'javelin-behavior-phabricator-object-selector' => '322728f3',
     'javelin-behavior-phabricator-oncopy' => 'f24c209c',
     'javelin-behavior-phabricator-remarkup-assist' => 'f24c209c',
     'javelin-behavior-phabricator-reveal-content' => 'f24c209c',
     'javelin-behavior-phabricator-search-typeahead' => 'f24c209c',
     'javelin-behavior-phabricator-tooltips' => 'f24c209c',
     'javelin-behavior-phabricator-watch-anchor' => 'f24c209c',
     'javelin-behavior-refresh-csrf' => 'f24c209c',
-    'javelin-behavior-repository-crossreference' => 'd2447f72',
+    'javelin-behavior-repository-crossreference' => '322728f3',
     'javelin-behavior-toggle-class' => 'f24c209c',
     'javelin-behavior-workflow' => 'f24c209c',
     'javelin-dom' => 'cd1d650a',
@@ -3814,7 +3828,7 @@
     'phabricator-core-css' => '09aa1f68',
     'phabricator-crumbs-view-css' => '09aa1f68',
     'phabricator-directory-css' => '09aa1f68',
-    'phabricator-drag-and-drop-file-upload' => 'd2447f72',
+    'phabricator-drag-and-drop-file-upload' => '322728f3',
     'phabricator-dropdown-menu' => 'f24c209c',
     'phabricator-file-upload' => 'f24c209c',
     'phabricator-filetree-view-css' => '09aa1f68',
@@ -3836,7 +3850,7 @@
     'phabricator-prefab' => 'f24c209c',
     'phabricator-project-tag-css' => 'e30a3fa8',
     'phabricator-remarkup-css' => '09aa1f68',
-    'phabricator-shaped-request' => 'd2447f72',
+    'phabricator-shaped-request' => '322728f3',
     'phabricator-side-menu-view-css' => '09aa1f68',
     'phabricator-standard-page-view' => '09aa1f68',
     'phabricator-textareautils' => 'f24c209c',

src/applications/diffusion/controller/DiffusionBrowseFileController.php

@@ -35,7 +35,10 @@ public function processRequest() {
         ->setURI($request->getRequestURI()->alter('view', $selected));
     }
 
-    $needs_blame = ($selected == 'blame' || $selected == 'plainblame');
+    $needs_blame = ($selected == 'plainblame');
+    if ($selected == 'blame' && $request->isAjax()) {
+      $needs_blame = true;
+    }
 
     $file_query = DiffusionFileContentQuery::newFromDiffusionRequest(
       $this->diffusionRequest);
@@ -59,6 +62,10 @@ public function processRequest() {
       $path,
       $data);
 
+    if ($request->isAjax()) {
+      return id(new AphrontAjaxResponse())->setContent($corpus);
+    }
+
     require_celerity_resource('diffusion-source-css');
 
     if ($this->corpusType == 'text') {
@@ -229,6 +236,18 @@ private function buildCorpus($selected,
         $rows = $this->buildDisplayRows($text_list, $rev_list, $blame_dict,
           $needs_blame, $drequest, $file_query, $selected);
 
+        $corpus_table = javelin_tag(
+          'table',
+          array(
+            'class' => "diffusion-source remarkup-code PhabricatorMonospaced",
+            'sigil' => 'diffusion-source',
+          ),
+          $rows);
+
+        if ($this->getRequest()->isAjax()) {
+          return $corpus_table;
+        }
+
         $id = celerity_generate_unique_node_id();
 
         $projects = $drequest->loadArcanistProjects();
@@ -260,14 +279,6 @@ private function buildCorpus($selected,
             ));
         }
 
-        $corpus_table = javelin_tag(
-          'table',
-          array(
-            'class' => "diffusion-source remarkup-code PhabricatorMonospaced",
-            'sigil' => 'diffusion-source',
-          ),
-          $rows);
-
         $corpus = phutil_tag(
           'div',
           array(
@@ -276,6 +287,8 @@ private function buildCorpus($selected,
           ),
           $corpus_table);
 
+        Javelin::initBehavior('load-blame', array('id' => $id));
+
         break;
     }
 
@@ -449,7 +462,6 @@ private function buildDisplayRows(
     $color = null;
     foreach ($text_list as $k => $line) {
       $display_line = array(
-        'color'       => null,
         'epoch'       => null,
         'commit'      => null,
         'author'      => null,
@@ -459,7 +471,7 @@ private function buildDisplayRows(
         'data'        => $line,
       );
 
-      if ($needs_blame) {
+      if ($selected == 'blame') {
         // If the line's rev is same as the line above, show empty content
         // with same color; otherwise generate blame info. The newer a change
         // is, the more saturated the color.
@@ -568,7 +580,7 @@ private function buildDisplayRows(
 
     $rows = $this->renderInlines(
       idx($inlines, 0, array()),
-      $needs_blame,
+      ($selected == 'blame'),
       $engine);
 
     foreach ($display as $line) {
@@ -581,8 +593,11 @@ private function buildDisplayRows(
         ));
 
       $blame = array();
-      if ($line['color']) {
-        $color = $line['color'];
+      $style = null;
+      if (array_key_exists('color', $line)) {
+        if ($line['color']) {
+          $style = 'background: '.$line['color'].';';
+        }
 
         $before_link = null;
         $commit_link = null;
@@ -667,31 +682,31 @@ private function buildDisplayRows(
           'th',
           array(
             'class' => 'diffusion-blame-link',
-            'style' => 'background: '.$color,
+            'style' => $style,
           ),
           $before_link);
 
         $blame[] = phutil_tag(
           'th',
           array(
             'class' => 'diffusion-rev-link',
-            'style' => 'background: '.$color,
+            'style' => $style,
           ),
           $commit_link);
 
         $blame[] = phutil_tag(
           'th',
           array(
             'class' => 'diffusion-rev-link',
-            'style' => 'background: '.$color,
+            'style' => $style,
           ),
           $revision_link);
 
         $blame[] = phutil_tag(
           'th',
           array(
             'class' => 'diffusion-author-link',
-            'style' => 'background: '.$color,
+            'style' => $style,
           ),
           idx($line, 'author'));
       }
@@ -708,7 +723,7 @@ private function buildDisplayRows(
         array(
           'class' => 'diffusion-line-link',
           'sigil' => 'diffusion-line-link',
-          'style' => isset($color) ? 'background: '.$color : null,
+          'style' => $style,
         ),
         $line_link);
 
@@ -753,7 +768,7 @@ private function buildDisplayRows(
 
       $rows = array_merge($rows, $this->renderInlines(
         idx($inlines, $line['line'], array()),
-        $needs_blame,
+        ($selected == 'blame'),
         $engine));
     }
 

webroot/rsrc/css/application/diffusion/diffusion-source.css

@@ -45,6 +45,18 @@
   white-space: nowrap;
 }
 
+.diffusion-blame-link {
+  min-width: 25px;
+}
+
+.diffusion-rev-link {
+  min-width: 90px;
+}
+
+.diffusion-author-link {
+  min-width: 120px;
+}
+
 .diffusion-blame-link a,
 .diffusion-rev-link a,
 .diffusion-author-link a,

webroot/rsrc/js/application/diffusion/behavior-load-blame.js

@@ -0,0 +1,14 @@
+/**
+ * @provides javelin-behavior-load-blame
+ * @requires javelin-behavior
+ *           javelin-dom
+ *           javelin-request
+ */
+
+JX.behavior('load-blame', function(config) {
+
+  new JX.Request(location.href, function (response) {
+    JX.DOM.setContent(JX.$(config.id), JX.$H(response));
+  }).send();
+
+});