Bug 1080661 - The count of shared URLs should be increased only once per generated URL. r=MattN
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Thu, 09 Oct 2014 19:04:14 +0100
changeset 225599 1a999d18d96b27b4eae708b28ad78bb35e8c686e
parent 225598 eaa6723729002aa8a75e52f6e5d782298aa16c73
child 225600 c3241c873fb2945d5c092b57c88c90a66cd35de8
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1080661
milestone34.0a2
Bug 1080661 - The count of shared URLs should be increased only once per generated URL. r=MattN
browser/components/loop/content/js/panel.js
browser/components/loop/content/js/panel.jsx
browser/components/loop/test/desktop-local/panel_test.js
--- a/browser/components/loop/content/js/panel.js
+++ b/browser/components/loop/content/js/panel.js
@@ -331,16 +331,19 @@ loop.panel = (function(_, mozL10n) {
       } else {
         try {
           var callUrl = new window.URL(callUrlData.callUrl);
           // XXX the current server vers does not implement the callToken field
           // but it exists in the API. This workaround should be removed in the future
           var token = callUrlData.callToken ||
                       callUrl.pathname.split('/').pop();
 
+          // Now that a new URL is available, indicate it has not been shared.
+          this.linkExfiltrated = false;
+
           this.setState({pending: false, copied: false,
                          callUrl: callUrl.href,
                          callUrlExpiry: callUrlData.expiresAt});
         } catch(e) {
           console.log(e);
           this.props.notifications.errorL10n("unable_retrieve_url");
           this.setState(this.getInitialState());
         }
@@ -357,22 +360,30 @@ loop.panel = (function(_, mozL10n) {
     handleCopyButtonClick: function(event) {
       this.handleLinkExfiltration(event);
       // XXX the mozLoop object should be passed as a prop, to ease testing and
       //     using a fake implementation in UI components showcase.
       navigator.mozLoop.copyString(this.state.callUrl);
       this.setState({copied: true});
     },
 
+    linkExfiltrated: false,
+
     handleLinkExfiltration: function(event) {
-      try {
-        navigator.mozLoop.telemetryAdd("LOOP_CLIENT_CALL_URL_SHARED", true);
-      } catch (err) {
-        console.error("Error recording telemetry", err);
+      // Update the count of shared URLs only once per generated URL.
+      if (!this.linkExfiltrated) {
+        this.linkExfiltrated = true;
+        try {
+          navigator.mozLoop.telemetryAdd("LOOP_CLIENT_CALL_URL_SHARED", true);
+        } catch (err) {
+          console.error("Error recording telemetry", err);
+        }
       }
+
+      // Note URL expiration every time it is shared.
       if (this.state.callUrlExpiry) {
         navigator.mozLoop.noteCallUrlExpiry(this.state.callUrlExpiry);
       }
     },
 
     render: function() {
       // XXX setting elem value from a state (in the callUrl input)
       // makes it immutable ie read only but that is fine in our case.
--- a/browser/components/loop/content/js/panel.jsx
+++ b/browser/components/loop/content/js/panel.jsx
@@ -331,16 +331,19 @@ loop.panel = (function(_, mozL10n) {
       } else {
         try {
           var callUrl = new window.URL(callUrlData.callUrl);
           // XXX the current server vers does not implement the callToken field
           // but it exists in the API. This workaround should be removed in the future
           var token = callUrlData.callToken ||
                       callUrl.pathname.split('/').pop();
 
+          // Now that a new URL is available, indicate it has not been shared.
+          this.linkExfiltrated = false;
+
           this.setState({pending: false, copied: false,
                          callUrl: callUrl.href,
                          callUrlExpiry: callUrlData.expiresAt});
         } catch(e) {
           console.log(e);
           this.props.notifications.errorL10n("unable_retrieve_url");
           this.setState(this.getInitialState());
         }
@@ -357,22 +360,30 @@ loop.panel = (function(_, mozL10n) {
     handleCopyButtonClick: function(event) {
       this.handleLinkExfiltration(event);
       // XXX the mozLoop object should be passed as a prop, to ease testing and
       //     using a fake implementation in UI components showcase.
       navigator.mozLoop.copyString(this.state.callUrl);
       this.setState({copied: true});
     },
 
+    linkExfiltrated: false,
+
     handleLinkExfiltration: function(event) {
-      try {
-        navigator.mozLoop.telemetryAdd("LOOP_CLIENT_CALL_URL_SHARED", true);
-      } catch (err) {
-        console.error("Error recording telemetry", err);
+      // Update the count of shared URLs only once per generated URL.
+      if (!this.linkExfiltrated) {
+        this.linkExfiltrated = true;
+        try {
+          navigator.mozLoop.telemetryAdd("LOOP_CLIENT_CALL_URL_SHARED", true);
+        } catch (err) {
+          console.error("Error recording telemetry", err);
+        }
       }
+
+      // Note URL expiration every time it is shared.
       if (this.state.callUrlExpiry) {
         navigator.mozLoop.noteCallUrlExpiry(this.state.callUrlExpiry);
       }
     },
 
     render: function() {
       // XXX setting elem value from a state (in the callUrl input)
       // makes it immutable ie read only but that is fine in our case.
--- a/browser/components/loop/test/desktop-local/panel_test.js
+++ b/browser/components/loop/test/desktop-local/panel_test.js
@@ -436,16 +436,18 @@ describe("loop.panel", function() {
           }));
           view.setState({
             pending: false,
             copied: false,
             callUrl: "http://example.com",
             callUrlExpiry: 6000
           });
 
+          // Multiple clicks should result in the URL being counted only once.
+          TestUtils.Simulate.click(view.getDOMNode().querySelector(".button-copy"));
           TestUtils.Simulate.click(view.getDOMNode().querySelector(".button-copy"));
 
           sinon.assert.calledOnce(navigator.mozLoop.telemetryAdd);
           sinon.assert.calledWith(navigator.mozLoop.telemetryAdd,
                                   "LOOP_CLIENT_CALL_URL_SHARED",
                                   true);
         });
 
@@ -477,16 +479,18 @@ describe("loop.panel", function() {
           }));
           view.setState({
             pending: false,
             copied: false,
             callUrl: "http://example.com",
             callUrlExpiry: 6000
           });
 
+          // Multiple clicks should result in the URL being counted only once.
+          TestUtils.Simulate.click(view.getDOMNode().querySelector(".button-email"));
           TestUtils.Simulate.click(view.getDOMNode().querySelector(".button-email"));
 
           sinon.assert.calledOnce(navigator.mozLoop.telemetryAdd);
           sinon.assert.calledWith(navigator.mozLoop.telemetryAdd,
                                   "LOOP_CLIENT_CALL_URL_SHARED",
                                   true);
         });
 
@@ -519,18 +523,20 @@ describe("loop.panel", function() {
           }));
           view.setState({
             pending: false,
             copied: false,
             callUrl: "http://example.com",
             callUrlExpiry: 6000
           });
 
+          // Multiple copies should result in the URL being counted only once.
           var urlField = view.getDOMNode().querySelector("input[type='url']");
           TestUtils.Simulate.copy(urlField);
+          TestUtils.Simulate.copy(urlField);
 
           sinon.assert.calledOnce(navigator.mozLoop.telemetryAdd);
           sinon.assert.calledWith(navigator.mozLoop.telemetryAdd,
                                   "LOOP_CLIENT_CALL_URL_SHARED",
                                   true);
         });
 
       it("should notify the user when the operation failed", function() {