Dashboard Cleanup: Add isLoadingComplete to the loader.Loader object.

https://bugs.webkit.org/show_bug.cgi?id=106247

Old code nulled out the loader instance when it completed loading and
then later used the fact that it was null to determine if it had loaded
or not.  This is not only unintuitive, but it also prevents using the
loader object later on.

Added new method, used it, added unit test for it.

Reviewed by Dirk Pranke.

* TestResultServer/static-dashboards/dashboard_base.js:
(resourceLoadingComplete):
(handleLocationChange):
* TestResultServer/static-dashboards/loader.js:
(.):
* TestResultServer/static-dashboards/loader_unittests.js:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@139471 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 011dffb..9787a51 100644 (file)
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,24 @@
+2013-01-11  Julie Parent  <jparent@chromium.org>
+
+        Dashboard Cleanup: Add isLoadingComplete to the loader.Loader object.
+        https://bugs.webkit.org/show_bug.cgi?id=106247
+
+        Old code nulled out the loader instance when it completed loading and
+        then later used the fact that it was null to determine if it had loaded
+        or not.  This is not only unintuitive, but it also prevents using the
+        loader object later on.
+        
+        Added new method, used it, added unit test for it.
+        
+        Reviewed by Dirk Pranke.
+
+        * TestResultServer/static-dashboards/dashboard_base.js:
+        (resourceLoadingComplete):
+        (handleLocationChange):
+        * TestResultServer/static-dashboards/loader.js:
+        (.):
+        * TestResultServer/static-dashboards/loader_unittests.js:
+
 2013-01-11  Ryosuke Niwa  <rniwa@webkit.org>
 
         Try CRLF to LF change in r139407 again.

diff --git a/Tools/TestResultServer/static-dashboards/dashboard_base.js b/Tools/TestResultServer/static-dashboards/dashboard_base.js
index 1b53ca2..35093e2 100644 (file)
--- a/Tools/TestResultServer/static-dashboards/dashboard_base.js
+++ b/Tools/TestResultServer/static-dashboards/dashboard_base.js
@@ -500,8 +500,6 @@ function showErrors()
 
 function resourceLoadingComplete(errorMsgs)
 {
-    g_resourceLoader = null;
-    
     if (errorMsgs)
         addError(errorMsgs)
 
@@ -510,7 +508,7 @@ function resourceLoadingComplete(errorMsgs)
 
 function handleLocationChange()
 {
-    if (g_resourceLoader)
+    if (!g_resourceLoader.isLoadingComplete())
         return;
 
     if (parseParameters())

diff --git a/Tools/TestResultServer/static-dashboards/loader.js b/Tools/TestResultServer/static-dashboards/loader.js
index ed3c4f7..39d4580 100644 (file)
--- a/Tools/TestResultServer/static-dashboards/loader.js
+++ b/Tools/TestResultServer/static-dashboards/loader.js
@@ -67,6 +67,7 @@ loader.Loader = function()
 
     this._buildersThatFailedToLoad = [];
     this._staleBuilders = [];
+    this._loadingComplete = false;
 }
 
 loader.Loader.prototype = {
@@ -74,10 +75,18 @@ loader.Loader.prototype = {
     {
         this._loadNext();
     },
+    isLoadingComplete: function()
+    {
+        return this._loadingComplete;
+    },
     _loadNext: function()
     {
         var loadingStep = this._loadingSteps.shift();
         if (!loadingStep) {
+            this._loadingComplete = true;
+            // FIXME(jparent): Loader should not know about global
+            // functions, should use a callback or dispatch load
+            // event instead.
             resourceLoadingComplete(this._getLoadingErrorMessages());
             return;
         }

diff --git a/Tools/TestResultServer/static-dashboards/loader_unittests.js b/Tools/TestResultServer/static-dashboards/loader_unittests.js
index 86d7bee..c193845 100644 (file)
--- a/Tools/TestResultServer/static-dashboards/loader_unittests.js
+++ b/Tools/TestResultServer/static-dashboards/loader_unittests.js
@@ -153,4 +153,14 @@ test('addBuilderLoadErrors', 1, function() {
     resourceLoader._buildersThatFailedToLoad = ['builder1', 'builder2'];
     resourceLoader._staleBuilders = ['staleBuilder1'];
     equal(resourceLoader._getLoadingErrorMessages(), 'ERROR: Failed to get data from builder1,builder2.<br>ERROR: Data from staleBuilder1 is more than 1 day stale.<br>');
+});
+
+test('Loaded state set', 2, function() {
+    resetGlobals();
+  
+    var resourceLoader = new loader.Loader();
+    equal(false, resourceLoader.isLoadingComplete(), 'Before loading, loading is not complete');
+    resourceLoader._loadingSteps = [];
+    resourceLoader.load();
+    equal(true, resourceLoader.isLoadingComplete(), 'After loading, loading is complete');
 });
\ No newline at end of file