Minor changes to dashboard notifier interface to enable better session-tracking
in notifier implementation. Also added some stuff to
...dev.shell.BrowserChannelServerTest to verify that the integration with the
dashboard is behaving correctly.
Review at http://gwt-code-reviews.appspot.com/1450812
Review by: tobyr@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10299 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/shell/BrowserChannelServer.java b/dev/core/src/com/google/gwt/dev/shell/BrowserChannelServer.java
index a9a7324..fa5adf0 100644
--- a/dev/core/src/com/google/gwt/dev/shell/BrowserChannelServer.java
+++ b/dev/core/src/com/google/gwt/dev/shell/BrowserChannelServer.java
@@ -19,6 +19,7 @@
import com.google.gwt.core.ext.TreeLogger.HelpInfo;
import com.google.gwt.dev.shell.BrowserChannel.SessionHandler.ExceptionOrReturnValue;
import com.google.gwt.dev.shell.JsValue.DispatchObject;
+import com.google.gwt.dev.util.log.dashboard.DashboardNotifier;
import com.google.gwt.dev.util.log.dashboard.DashboardNotifierFactory;
import java.io.IOException;
@@ -150,7 +151,7 @@
this.ignoreRemoteDeath = ignoreRemoteDeath;
init(initialLogger);
}
-
+
/**
* Indicate that Java no longer has references to the supplied JS objects.
*
@@ -380,6 +381,7 @@
* @throws IOException
*/
public void shutdown() throws IOException {
+ getDashboardNotifier().devModeSessionEnded(devModeSession);
QuitMessage.send(this);
}
@@ -644,7 +646,16 @@
break;
}
}
-
+
+ /**
+ * Returns the {@code DashboardNotifier} used to send notices to a dashboard
+ * service.
+ */
+ // @VisibleForTesting
+ DashboardNotifier getDashboardNotifier() {
+ return DashboardNotifierFactory.getNotifier();
+ }
+
/**
* Creates the {@code DevModeSession} that represents the current browser
* connection, sets it as the "default" session for the current thread, and
@@ -653,7 +664,7 @@
private void createDevModeSession() {
devModeSession = new DevModeSession(moduleName, userAgent);
DevModeSession.setSessionForCurrentThread(devModeSession);
- DashboardNotifierFactory.getNotifier().devModeSession(devModeSession);
+ getDashboardNotifier().devModeSession(devModeSession);
}
/**
diff --git a/dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifier.java b/dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifier.java
index c89ddcc..a4eaebe 100644
--- a/dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifier.java
+++ b/dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifier.java
@@ -28,19 +28,24 @@
// TODO(jhumphries) Add interface methods for collecting data from the
// compiler
- // Second: Dev-Mode related entry points
+ // Second: Devmode related entry points
/**
- * Records a top-level event to the dashboard.
+ * Notifies the dashboard of a top-level event.
*/
void devModeEvent(DevModeSession session, String eventType, long startTimeNanos,
long durationNanos);
/**
- * Records a new module/session.
+ * Notifies the dashboard of a new session starting.
*/
void devModeSession(DevModeSession session);
+ /**
+ * Notifies the dashboard of a session ending.
+ */
+ void devModeSessionEnded(DevModeSession session);
+
// Third: Test related entry points
// TODO(jhumphries) Add interface methods for collecting data from automated
diff --git a/dev/core/src/com/google/gwt/dev/util/log/dashboard/NoOpDashboardNotifier.java b/dev/core/src/com/google/gwt/dev/util/log/dashboard/NoOpDashboardNotifier.java
index 844f59d..e6a8a30 100644
--- a/dev/core/src/com/google/gwt/dev/util/log/dashboard/NoOpDashboardNotifier.java
+++ b/dev/core/src/com/google/gwt/dev/util/log/dashboard/NoOpDashboardNotifier.java
@@ -34,4 +34,9 @@
// do nothing
}
+ @Override
+ public void devModeSessionEnded(DevModeSession session) {
+ // do nothing
+ }
+
}
diff --git a/dev/core/test/com/google/gwt/dev/shell/BrowserChannelServerTest.java b/dev/core/test/com/google/gwt/dev/shell/BrowserChannelServerTest.java
index 2653712..f43552f 100644
--- a/dev/core/test/com/google/gwt/dev/shell/BrowserChannelServerTest.java
+++ b/dev/core/test/com/google/gwt/dev/shell/BrowserChannelServerTest.java
@@ -27,6 +27,7 @@
import com.google.gwt.dev.shell.BrowserChannel.UserAgentIconMessage;
import com.google.gwt.dev.shell.BrowserChannel.Value;
import com.google.gwt.dev.shell.BrowserChannelServer.SessionHandlerServer;
+import com.google.gwt.dev.util.log.dashboard.DashboardNotifier;
import junit.framework.TestCase;
@@ -44,10 +45,14 @@
public class BrowserChannelServerTest extends TestCase {
/**
- * A BrowserChannelServer that can notify when the connection is closed.
+ * A BrowserChannelServer that can notify when the connection is closed. It
+ * also uses a custom {@code DashboardNotifier} in order to test notification
+ * calls.
*/
private class TestBrowserChannelServer extends BrowserChannelServer {
+ TestDashboardNotifier notifier = new TestDashboardNotifier();
+
private final Semaphore finishNotify = new Semaphore(0);
public TestBrowserChannelServer(TreeLogger logger,
@@ -57,12 +62,16 @@
}
@Override
- protected void processConnection() throws IOException,
- BrowserChannelException {
- super.processConnection();
+ public void run() {
+ super.run();
finishNotify.release();
}
+ @Override
+ DashboardNotifier getDashboardNotifier() {
+ return notifier;
+ }
+
public void waitForClose() throws InterruptedException {
finishNotify.acquire();
}
@@ -180,6 +189,45 @@
loadedModule = null;
}
}
+
+ /**
+ * A dashboard notifier that enforces the correct method calls.
+ */
+ private static class TestDashboardNotifier implements DashboardNotifier {
+ DevModeSession currentSession;
+ boolean started;
+ boolean ended;
+
+ @Override
+ public void devModeEvent(DevModeSession session, String eventType, long startTimeNanos,
+ long durationNanos) {
+ fail("BrowserChannelServer should not be calling DashboardNotifier.devModeEvent()");
+ }
+
+ @Override
+ public void devModeSession(DevModeSession session) {
+ currentSession = session;
+ assertFalse("DashboardNotifier.devModeSession() called more than once", started);
+ started = true;
+ }
+
+ @Override
+ public void devModeSessionEnded(DevModeSession session) {
+ assertTrue("DashboardNotifier.devModeSessionEnded() without prior call to "
+ + "DashboardNotifier.devModeSession()", started);
+ assertFalse("DashboardNotifier.devModeSessionEnded() called more than once", ended);
+ assertEquals("Wrong session passed to DashboardNotifier.devModeSessionEnded()",
+ currentSession, session);
+ ended = true;
+ }
+
+ public void verify(String moduleName, String userAgent) {
+ assertTrue(started);
+ assertTrue(ended);
+ assertEquals(moduleName, currentSession.getModuleName());
+ assertEquals(userAgent, currentSession.getUserAgent());
+ }
+ }
private PipedStreamPair clientToServer = new PipedStreamPair();
private PipedStreamPair serverToClient = new PipedStreamPair();
@@ -216,6 +264,7 @@
QuitMessage.send(client);
server.waitForClose();
assertNull(handler.getLoadedModule());
+ server.notifier.verify("testModule", "userAgent");
}
/**
@@ -258,6 +307,7 @@
QuitMessage.send(client);
server.waitForClose();
assertNull(handler.getLoadedModule());
+ server.notifier.verify("testModule", "userAgent");
}
/**
@@ -312,5 +362,6 @@
QuitMessage.send(client);
server.waitForClose();
assertNull(handler.getLoadedModule());
+ server.notifier.verify("testModule", "userAgent");
}
}
diff --git a/dev/core/test/com/google/gwt/dev/util/log/dashboard/DashboardNotifierFactoryTest.java b/dev/core/test/com/google/gwt/dev/util/log/dashboard/DashboardNotifierFactoryTest.java
index d3950dd..6765ea0 100644
--- a/dev/core/test/com/google/gwt/dev/util/log/dashboard/DashboardNotifierFactoryTest.java
+++ b/dev/core/test/com/google/gwt/dev/util/log/dashboard/DashboardNotifierFactoryTest.java
@@ -37,6 +37,11 @@
public void devModeSession(DevModeSession session) {
// no need to do anything
}
+
+ @Override
+ public void devModeSessionEnded(DevModeSession session) {
+ // no need to do anything
+ }
};
// call method
DashboardNotifierFactory.setNotifier(obj);
diff --git a/dev/core/test/com/google/gwt/dev/util/log/dashboard/SpeedTracerLoggerTestMockNotifier.java b/dev/core/test/com/google/gwt/dev/util/log/dashboard/SpeedTracerLoggerTestMockNotifier.java
index 1b7cd10..e6cd463 100644
--- a/dev/core/test/com/google/gwt/dev/util/log/dashboard/SpeedTracerLoggerTestMockNotifier.java
+++ b/dev/core/test/com/google/gwt/dev/util/log/dashboard/SpeedTracerLoggerTestMockNotifier.java
@@ -101,11 +101,17 @@
}
@Override
+ public void devModeSessionEnded(DevModeSession session) {
+ // always raise exception here - this method shouldn't be invoked from
+ // SpeedTracerLogger
+ Assert.fail("SpeedTracerLogger should not be calling DashboardNotifier.devModeSessionEnded()");
+ }
+
+ @Override
public void devModeSession(DevModeSession session) {
// always raise exception here - this method shouldn't be invoked from
// SpeedTracerLogger
- Assert.assertTrue("SpeedTracerLogger should not be calling DashboardNotifier.devModeSession()",
- false);
+ Assert.fail("SpeedTracerLogger should not be calling DashboardNotifier.devModeSession()");
}
public LinkedList<DevModeEvent> getEventSequence() {
