From 77f9b407f6e49e61e86caf85e737c3326f9af0bd Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Fri, 12 Feb 2016 16:34:43 -0800 Subject: [PATCH 01/34] Bug 1244295 - Create client ID if it doesn't already exist in GeckoProfile. r=mfinkle Additionally, we'll try to migrate the client ID from FHR if it doesn't already exist. MozReview-Commit-ID: B9vfefeVi2i --HG-- extra : rebase_source : f5acdba8f276f5cbc04a8877a0589f4ce8155e49 --- .../java/org/mozilla/gecko/GeckoProfile.java | 71 ++++++++++++++++++- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java b/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java index 38768141ec69..5734c43e6de9 100644 --- a/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java +++ b/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java @@ -16,6 +16,7 @@ import java.nio.charset.Charset; import java.util.Enumeration; import java.util.HashMap; import java.util.Hashtable; +import java.util.UUID; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -48,6 +49,7 @@ public final class GeckoProfile { // The path in the profile to the file containing the client ID. private static final String CLIENT_ID_FILE_PATH = "datareporting/state.json"; + private static final String FHR_CLIENT_ID_FILE_PATH = "healthreport/state.json"; // In the client ID file, the attribute title in the JSON object containing the client ID value. private static final String CLIENT_ID_JSON_ATTR = "clientID"; @@ -599,7 +601,8 @@ public final class GeckoProfile { } /** - * Retrieves the Gecko client ID from the filesystem. + * Retrieves the Gecko client ID from the filesystem. If the client ID does not exist, we attempt to migrate and + * persist it from FHR and, if that fails, we attempt to create a new one ourselves. * * This method assumes the client ID is located in a file at a hard-coded path within the profile. The format of * this file is a JSONObject which at the bottom level contains a String -> String mapping containing the client ID. @@ -613,10 +616,12 @@ public final class GeckoProfile { * TODO: Write tests to prevent regressions. Mention them here. Test both file location and file format. * * [1]: https://mxr.mozilla.org/mozilla-central/source/toolkit/modules/ClientID.jsm + * + * @throws IOException if the client ID could not be retrieved. */ @WorkerThread public String getClientId() throws IOException { - final JSONObject obj = readJSONObjectFromFile(CLIENT_ID_FILE_PATH); + final JSONObject obj = getClientIdJSONObject(); try { return obj.getString(CLIENT_ID_JSON_ATTR); } catch (final JSONException e) { @@ -625,6 +630,54 @@ public final class GeckoProfile { } } + // Mimics ClientID.jsm – _doLoadClientID. One exception is that we don't validate client IDs like it does. + @WorkerThread + private JSONObject getClientIdJSONObject() throws IOException { + try { + return readJSONObjectFromFile(CLIENT_ID_FILE_PATH); + } catch (final IOException e) { /* No contemporary client ID: fallthrough. */ } + + Log.d(LOGTAG, "Could not get client ID – attempting to migrate ID from FHR"); + String clientIdToWrite; + try { + final JSONObject fhrClientIdObj = readJSONObjectFromFile(FHR_CLIENT_ID_FILE_PATH); + clientIdToWrite = fhrClientIdObj.getString(CLIENT_ID_JSON_ATTR); + } catch (final IOException|JSONException e) { + Log.d(LOGTAG, "Could not migrate client ID from FHR – creating a new one"); + clientIdToWrite = UUID.randomUUID().toString(); + } + + // There is a possibility Gecko is running and the Gecko telemetry implementation decided it's time to generate + // the client ID, writing client ID underneath us. Since it's highly unlikely (e.g. we run in onStart before + // Gecko is started), we don't handle that possibility besides writing the ID and then reading from the file + // again (rather than just returning the value we generated before writing). + // + // In the event it does happen, any discrepancy will be resolved after a restart. In the mean time, both this + // implementation and the Gecko implementation could upload documents with inconsistent IDs. + // + // In any case, if we get an exception, intentionally throw - there's nothing more to do here. + persistClientId(clientIdToWrite); + return readJSONObjectFromFile(CLIENT_ID_FILE_PATH); + } + + @WorkerThread + private void persistClientId(final String clientId) throws IOException { + if (!ensureParentDirs(CLIENT_ID_FILE_PATH)) { + throw new IOException("Could not create client ID parent directories"); + } + + final JSONObject obj = new JSONObject(); + try { + obj.put(CLIENT_ID_JSON_ATTR, clientId); + } catch (final JSONException e) { + throw new IOException("Could not create client ID JSON object", e); + } + + // ClientID.jsm overwrites the file to store the client ID so it's okay if we do it too. + Log.d(LOGTAG, "Attempting to write new client ID"); + writeFile(CLIENT_ID_FILE_PATH, obj.toString()); // Logs errors within function: ideally we'd throw. + } + /** * @return the profile creation date in the format returned by {@link System#currentTimeMillis()} or -1 if the value * was not found. @@ -702,6 +755,20 @@ public final class GeckoProfile { return null; } + /** + * Ensures the parent director(y|ies) of the given filename exist by making them + * if they don't already exist.. + * + * @param filename The path to the file whose parents should be made directories + * @return true if the parent directory exists, false otherwise + */ + @WorkerThread + public boolean ensureParentDirs(final String filename) { + final File file = new File(getDir(), filename); + final File parentFile = file.getParentFile(); + return parentFile.mkdirs() || parentFile.isDirectory(); + } + public void writeFile(final String filename, final String data) { File file = new File(getDir(), filename); BufferedWriter bufferedWriter = null; From 7422c1d7ef83e5b189bc3584df2dcd03a42e25da Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Wed, 17 Feb 2016 12:22:18 -0800 Subject: [PATCH 02/34] Bug 1244295 - Validate client IDs before sending them in a Telemtry report. r=mfinkle Additionally, we log some of the Exceptions thrown while retrieving the client ID to make it clearer what is happening. The underlying GeckoProfile methods ensure the profile path is not printed so we don't have to worry about leaking that. MozReview-Commit-ID: 3o0rvXDRZzM --HG-- extra : rebase_source : 81cff9a199365623b16efe21a12509b8bf7b3c6e --- .../java/org/mozilla/gecko/GeckoProfile.java | 52 ++++++++++++------- .../telemetry/TelemetryUploadService.java | 3 +- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java b/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java index 5734c43e6de9..ddc175edcb42 100644 --- a/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java +++ b/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java @@ -619,31 +619,22 @@ public final class GeckoProfile { * * @throws IOException if the client ID could not be retrieved. */ + // Mimics ClientID.jsm – _doLoadClientID. @WorkerThread public String getClientId() throws IOException { - final JSONObject obj = getClientIdJSONObject(); try { - return obj.getString(CLIENT_ID_JSON_ATTR); - } catch (final JSONException e) { - // Don't log to avoid leaking data in JSONObject. - throw new IOException("Client ID does not exist in JSONObject"); + return getValidClientIdFromDisk(CLIENT_ID_FILE_PATH); + } catch (final IOException e) { + // Avoid log spam: don't log the full Exception w/ the stack trace. + Log.d(LOGTAG, "Could not get client ID - attempting to migrate ID from FHR: " + e.getLocalizedMessage()); } - } - // Mimics ClientID.jsm – _doLoadClientID. One exception is that we don't validate client IDs like it does. - @WorkerThread - private JSONObject getClientIdJSONObject() throws IOException { - try { - return readJSONObjectFromFile(CLIENT_ID_FILE_PATH); - } catch (final IOException e) { /* No contemporary client ID: fallthrough. */ } - - Log.d(LOGTAG, "Could not get client ID – attempting to migrate ID from FHR"); String clientIdToWrite; try { - final JSONObject fhrClientIdObj = readJSONObjectFromFile(FHR_CLIENT_ID_FILE_PATH); - clientIdToWrite = fhrClientIdObj.getString(CLIENT_ID_JSON_ATTR); - } catch (final IOException|JSONException e) { - Log.d(LOGTAG, "Could not migrate client ID from FHR – creating a new one"); + clientIdToWrite = getValidClientIdFromDisk(FHR_CLIENT_ID_FILE_PATH); + } catch (final IOException e) { + // Avoid log spam: don't log the full Exception w/ the stack trace. + Log.d(LOGTAG, "Could not migrate client ID from FHR – creating a new one: " + e.getLocalizedMessage()); clientIdToWrite = UUID.randomUUID().toString(); } @@ -657,7 +648,21 @@ public final class GeckoProfile { // // In any case, if we get an exception, intentionally throw - there's nothing more to do here. persistClientId(clientIdToWrite); - return readJSONObjectFromFile(CLIENT_ID_FILE_PATH); + return getValidClientIdFromDisk(CLIENT_ID_FILE_PATH); + } + + /** + * @return a valid client ID + * @throws IOException if a valid client ID could not be retrieved + */ + @WorkerThread + private String getValidClientIdFromDisk(final String filePath) throws IOException { + final JSONObject obj = readJSONObjectFromFile(filePath); + final String clientId = obj.optString(CLIENT_ID_JSON_ATTR); + if (isClientIdValid(clientId)) { + return clientId; + } + throw new IOException("Received client ID is invalid: " + clientId); } @WorkerThread @@ -678,6 +683,15 @@ public final class GeckoProfile { writeFile(CLIENT_ID_FILE_PATH, obj.toString()); // Logs errors within function: ideally we'd throw. } + // From ClientID.jsm - isValidClientID. + public static boolean isClientIdValid(final String clientId) { + // We could use UUID.fromString but, for consistency, we take the implementation from ClientID.jsm. + if (TextUtils.isEmpty(clientId)) { + return false; + } + return clientId.matches("(?i:[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})"); + } + /** * @return the profile creation date in the format returned by {@link System#currentTimeMillis()} or -1 if the value * was not found. diff --git a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java index 384870414c59..9eee05cb17cb 100644 --- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java +++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java @@ -172,8 +172,7 @@ public class TelemetryUploadService extends BackgroundService { try { clientId = profile.getClientId(); } catch (final IOException e) { - // Don't log the exception to avoid leaking the profile path. - Log.w(LOGTAG, "Unable to get client ID to generate core ping: returning."); + Log.w(LOGTAG, "Unable to get client ID to generate core ping: returning.", e); return; } From 9b94529a7dd8686313e13ce4f2258e90e83a88fd Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Thu, 18 Feb 2016 17:38:16 -0800 Subject: [PATCH 03/34] Bug 1244295 - Add junit4 tests for GeckoProfile.getClient & friends. r=mfinkle Added testGetDir to sanity check how the profile is set up for the test and left it in as a bonus. Additionally, changed access levels on the ensureParentDirs method because it only needed to be `protected` for testing. MozReview-Commit-ID: CDVQjyf3yP2 --HG-- extra : rebase_source : e427eb8356ac71adb8a672869ee297a0ffc5b1dc --- .../java/org/mozilla/gecko/GeckoProfile.java | 7 +- .../org/mozilla/gecko/TestGeckoProfile.java | 204 ++++++++++++++++++ .../org/mozilla/gecko/helpers/FileUtil.java | 42 ++++ 3 files changed, 249 insertions(+), 4 deletions(-) create mode 100644 mobile/android/tests/background/junit4/src/org/mozilla/gecko/TestGeckoProfile.java create mode 100644 mobile/android/tests/background/junit4/src/org/mozilla/gecko/helpers/FileUtil.java diff --git a/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java b/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java index ddc175edcb42..036721666f09 100644 --- a/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java +++ b/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java @@ -611,9 +611,8 @@ public final class GeckoProfile { * robust way to access it. However, we don't want to rely on Gecko running in order to get * the client ID so instead we access the file this module accesses directly. However, it's * possible the format of this file (and the access calls in the jsm) will change, leaving - * this code to fail. - * - * TODO: Write tests to prevent regressions. Mention them here. Test both file location and file format. + * this code to fail. There are tests in TestGeckoProfile to verify the file format but be + * warned: THIS IS NOT FOOLPROOF. * * [1]: https://mxr.mozilla.org/mozilla-central/source/toolkit/modules/ClientID.jsm * @@ -777,7 +776,7 @@ public final class GeckoProfile { * @return true if the parent directory exists, false otherwise */ @WorkerThread - public boolean ensureParentDirs(final String filename) { + protected boolean ensureParentDirs(final String filename) { final File file = new File(getDir(), filename); final File parentFile = file.getParentFile(); return parentFile.mkdirs() || parentFile.isDirectory(); diff --git a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/TestGeckoProfile.java b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/TestGeckoProfile.java new file mode 100644 index 000000000000..d5307239562e --- /dev/null +++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/TestGeckoProfile.java @@ -0,0 +1,204 @@ +/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*- + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.gecko; + +import android.content.Context; + +import org.json.JSONObject; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.mozilla.gecko.background.testhelpers.TestRunner; +import org.mozilla.gecko.helpers.FileUtil; +import org.robolectric.RuntimeEnvironment; + +import java.io.File; +import java.io.IOException; +import java.util.UUID; + +import static org.junit.Assert.*; + +/** + * Unit test methods of the GeckoProfile class. + */ +@RunWith(TestRunner.class) +public class TestGeckoProfile { + private static final String PROFILE_NAME = "profileName"; + + private static final String CLIENT_ID_JSON_ATTR = "clientID"; + + @Rule + public TemporaryFolder dirContainingProfile = new TemporaryFolder(); + + private File profileDir; + private GeckoProfile profile; + + private File clientIdFile; + + @Before + public void setUp() throws IOException { + final Context context = RuntimeEnvironment.application; + profileDir = dirContainingProfile.newFolder(); + profile = GeckoProfile.get(context, PROFILE_NAME, profileDir); + + clientIdFile = new File(profileDir, "datareporting/state.json"); + } + + public void assertValidClientId(final String clientId) { + // This isn't the method we use in the main GeckoProfile code, but it should be equivalent. + UUID.fromString(clientId); // assert: will throw if null or invalid UUID. + } + + @Test + public void testGetDir() { + assertEquals("Profile dir argument during construction and returned value are equal", + profileDir, profile.getDir()); + } + + @Test + public void testGetClientIdFreshProfile() throws Exception { + assertFalse("client ID file does not exist", clientIdFile.exists()); + + // No existing client ID file: we're expected to create one. + final String clientId = profile.getClientId(); + assertValidClientId(clientId); + assertTrue("client ID file exists", clientIdFile.exists()); + + assertEquals("Returned client ID is the same as the one previously returned", clientId, profile.getClientId()); + assertEquals("clientID file format matches expectations", clientId, readClientIdFromFile(clientIdFile)); + } + + @Test + public void testGetClientIdMigrateFromFHR() throws Exception { + final File fhrClientIdFile = new File(profileDir, "healthreport/state.json"); + final String fhrClientId = "905de1c0-0ea6-4a43-95f9-6170035f5a82"; + + assertFalse("client ID file does not exist", clientIdFile.exists()); + assertTrue("Created FHR data directory", new File(profileDir, "healthreport").mkdirs()); + writeClientIdToFile(fhrClientIdFile, fhrClientId); + assertEquals("Migrated Client ID equals FHR client ID", fhrClientId, profile.getClientId()); + + // Verify migration wrote to contemporary client ID file. + assertTrue("Client ID file created during migration", clientIdFile.exists()); + assertEquals("Migrated client ID on disk equals value returned from method", + fhrClientId, readClientIdFromFile(clientIdFile)); + + assertTrue("Deleted FHR clientID file", fhrClientIdFile.delete()); + assertEquals("Ensure method calls read from newly created client ID file & not FHR client ID file", + fhrClientId, profile.getClientId()); + } + + @Test + public void testGetClientIdInvalidIdOnDisk() throws Exception { + assertTrue("Created the parent dirs of the client ID file", clientIdFile.getParentFile().mkdirs()); + writeClientIdToFile(clientIdFile, ""); + final String clientIdForEmptyString = profile.getClientId(); + assertValidClientId(clientIdForEmptyString); + assertNotEquals("A new client ID was created when the empty String was written to disk", "", clientIdForEmptyString); + + writeClientIdToFile(clientIdFile, "invalidClientId"); + final String clientIdForInvalidClientId = profile.getClientId(); + assertValidClientId(clientIdForInvalidClientId); + assertNotEquals("A new client ID was created when an invalid client ID was written to disk", + "invalidClientId", clientIdForInvalidClientId); + } + + @Test + public void testGetClientIdMissingClientIdJSONAttr() throws Exception { + final String validClientId = "905de1c0-0ea6-4a43-95f9-6170035f5a82"; + final JSONObject objMissingClientId = new JSONObject(); + objMissingClientId.put("irrelevantKey", validClientId); + assertTrue("Created the parent dirs of the client ID file", clientIdFile.getParentFile().mkdirs()); + FileUtil.writeJSONObjectToFile(clientIdFile, objMissingClientId); + + final String clientIdForMissingAttr = profile.getClientId(); + assertValidClientId(clientIdForMissingAttr); + assertNotEquals("Did not use other attr when JSON attr was missing", validClientId, clientIdForMissingAttr); + } + + @Test + public void testGetClientIdInvalidIdFileFormat() throws Exception { + final String validClientId = "905de1c0-0ea6-4a43-95f9-6170035f5a82"; + assertTrue("Created the parent dirs of the client ID file", clientIdFile.getParentFile().mkdirs()); + FileUtil.writeStringToFile(clientIdFile, "clientID: \"" + validClientId + "\""); + + final String clientIdForInvalidFormat = profile.getClientId(); + assertValidClientId(clientIdForInvalidFormat); + assertNotEquals("Created new ID when file format was invalid", validClientId, clientIdForInvalidFormat); + } + + @Test + public void testEnsureParentDirs() { + final File grandParentDir = new File(profileDir, "grandParent"); + final File parentDir = new File(grandParentDir, "parent"); + final File childFile = new File(parentDir, "child"); + + // Assert initial state. + assertFalse("Topmost parent dir should not exist yet", grandParentDir.exists()); + assertFalse("Bottommost parent dir should not exist yet", parentDir.exists()); + assertFalse("Child file should not exist", childFile.exists()); + + final String fakeFullPath = "grandParent/parent/child"; + assertTrue("Parent directories should be created", profile.ensureParentDirs(fakeFullPath)); + assertTrue("Topmost parent dir should have been created", grandParentDir.exists()); + assertTrue("Bottommost parent dir should have been created", parentDir.exists()); + assertFalse("Child file should not have been created", childFile.exists()); + + // Parents already exist because this is the second time we're calling ensureParentDirs. + assertTrue("Expect true if parent directories already exist", profile.ensureParentDirs(fakeFullPath)); + + // Assert error condition. + assertTrue("Ensure we can change permissions on profile dir for testing", profileDir.setReadOnly()); + assertFalse("Expect false if the parent dir could not be created", profile.ensureParentDirs("unwritableDir/child")); + } + + @Test + public void testIsClientIdValid() { + final String[] validClientIds = new String[] { + "905de1c0-0ea6-4a43-95f9-6170035f5a82", + "905de1c0-0ea6-4a43-95f9-6170035f5a83", + "57472f82-453d-4c55-b59c-d3c0e97b76a1", + "895745d1-f31e-46c3-880e-b4dd72963d4f", + }; + for (final String validClientId : validClientIds) { + assertTrue("Client ID, " + validClientId + ", is valid", profile.isClientIdValid(validClientId)); + } + + final String[] invalidClientIds = new String[] { + null, + "", + "a", + "anInvalidClientId", + "905de1c0-0ea6-4a43-95f9-6170035f5a820", // too long (last section) + "905de1c0-0ea6-4a43-95f9-6170035f5a8", // too short (last section) + "05de1c0-0ea6-4a43-95f9-6170035f5a82", // too short (first section) + "905de1c0-0ea6-4a43-95f9-6170035f5a8!", // contains a symbol + }; + for (final String invalidClientId : invalidClientIds) { + assertFalse("Client ID, " + invalidClientId + ", is invalid", profile.isClientIdValid(invalidClientId)); + } + + // We generate client IDs using UUID - better make sure they're valid. + for (int i = 0; i < 30; ++i) { + final String generatedClientId = UUID.randomUUID().toString(); + assertTrue("Generated client ID from UUID, " + generatedClientId + ", is valid", + profile.isClientIdValid(generatedClientId)); + } + } + + private String readClientIdFromFile(final File file) throws Exception { + final JSONObject obj = FileUtil.readJSONObjectFromFile(file); + return obj.getString(CLIENT_ID_JSON_ATTR); + } + + private void writeClientIdToFile(final File file, final String clientId) throws Exception { + final JSONObject obj = new JSONObject(); + obj.put(CLIENT_ID_JSON_ATTR, clientId); + FileUtil.writeJSONObjectToFile(file, obj); + } +} diff --git a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/helpers/FileUtil.java b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/helpers/FileUtil.java new file mode 100644 index 000000000000..fb29f3621f67 --- /dev/null +++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/helpers/FileUtil.java @@ -0,0 +1,42 @@ +/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*- + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.gecko.helpers; + +import org.json.JSONObject; + +import java.io.File; +import java.io.FileWriter; +import java.util.Scanner; + +public class FileUtil { + private FileUtil() { } + + public static JSONObject readJSONObjectFromFile(final File file) throws Exception { + final StringBuilder builder = new StringBuilder(); + final Scanner scanner = new Scanner(file); + try { + while (scanner.hasNext()) { + builder.append(scanner.next()); + } + } finally { + scanner.close(); + } + return new JSONObject(builder.toString()); + } + + public static void writeJSONObjectToFile(final File file, final JSONObject obj) throws Exception { + writeStringToFile(file, obj.toString()); + } + + public static void writeStringToFile(final File file, final String str) throws Exception { + final FileWriter writer = new FileWriter(file, false); + try { + writer.write(str); + } finally { + writer.close(); + } + } +} From 661e9433e3a5d965891f4bd391fe7b912088c63f Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Wed, 17 Feb 2016 15:31:26 -0800 Subject: [PATCH 04/34] Bug 1244295 - Add regression warning to ClientID.jsm getClientID method. r=gfritzsche MozReview-Commit-ID: Eci1sG9HPem --HG-- extra : rebase_source : ef4201e381f5f268d02f73854bd78d86c26aeb59 --- toolkit/modules/ClientID.jsm | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/toolkit/modules/ClientID.jsm b/toolkit/modules/ClientID.jsm index c557e220eda0..019b19c79627 100644 --- a/toolkit/modules/ClientID.jsm +++ b/toolkit/modules/ClientID.jsm @@ -48,6 +48,11 @@ this.ClientID = Object.freeze({ * data reporting (FHR & Telemetry). Previously exising FHR client IDs are * migrated to this. * + * WARNING: This functionality is duplicated for Android (see GeckoProfile.getClientId + * for more). There are Java tests (TestGeckoProfile) to ensure the functionality is + * consistent and Gecko tests to come (bug 1249156). However, THIS IS NOT FOOLPROOF. + * Be careful when changing this code and, in particular, the underlying file format. + * * @return {Promise} The stable client ID. */ getClientID: function() { From 62759f5c7bedaa8810fc91e72754ed936ede6b0d Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Thu, 18 Feb 2016 17:46:52 -0800 Subject: [PATCH 05/34] Bug 1244295 - Add getClientId test for when client ID file already exists. r=me Similar to gfritzsche's suggestion in bug 1244295 comment 26. MozReview-Commit-ID: Agqyj47uSZR --HG-- extra : rebase_source : 24032f7cd46e7c2cd7143dc822677e51d5445cc6 --- .../junit4/src/org/mozilla/gecko/TestGeckoProfile.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/TestGeckoProfile.java b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/TestGeckoProfile.java index d5307239562e..8069b0ebee26 100644 --- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/TestGeckoProfile.java +++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/TestGeckoProfile.java @@ -73,6 +73,16 @@ public class TestGeckoProfile { assertEquals("clientID file format matches expectations", clientId, readClientIdFromFile(clientIdFile)); } + @Test + public void testGetClientIdFileAlreadyExists() throws Exception { + final String validClientId = "905de1c0-0ea6-4a43-95f9-6170035f5a82"; + assertTrue("Created the parent dirs of the client ID file", clientIdFile.getParentFile().mkdirs()); + writeClientIdToFile(clientIdFile, validClientId); + + final String clientIdFromProfile = profile.getClientId(); + assertEquals("Client ID from method matches ID written to disk", validClientId, clientIdFromProfile); + } + @Test public void testGetClientIdMigrateFromFHR() throws Exception { final File fhrClientIdFile = new File(profileDir, "healthreport/state.json"); From 80f028f5b47c6ffb3667cb605f39d3bfa48f045e Mon Sep 17 00:00:00 2001 From: Maurya Talisetti Date: Thu, 18 Feb 2016 08:57:00 +0100 Subject: [PATCH 06/34] Bug 1156176 - Add Restart Later button for the setting that allows Dev Edition and Firefox to run at the same time. r=jaws --- .../components/preferences/in-content/main.js | 70 +++++++++++++------ .../preferences/preferences.properties | 3 + 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/browser/components/preferences/in-content/main.js b/browser/components/preferences/in-content/main.js index 7f109d5154bb..ea63f1257562 100644 --- a/browser/components/preferences/in-content/main.js +++ b/browser/components/preferences/in-content/main.js @@ -7,6 +7,10 @@ Components.utils.import("resource://gre/modules/FileUtils.jsm"); Components.utils.import("resource://gre/modules/Task.jsm"); Components.utils.import("resource:///modules/ShellService.jsm"); Components.utils.import("resource:///modules/TransientPrefs.jsm"); + +XPCOMUtils.defineLazyModuleGetter(this, "OS", + "resource://gre/modules/osfile.jsm"); + #ifdef E10S_TESTING_ONLY XPCOMUtils.defineLazyModuleGetter(this, "UpdateUtils", "resource://gre/modules/UpdateUtils.jsm"); @@ -101,7 +105,6 @@ var gMainPane = { #endif #ifdef MOZ_DEV_EDITION - Cu.import("resource://gre/modules/osfile.jsm"); let uAppData = OS.Constants.Path.userApplicationDataDir; let ignoreSeparateProfile = OS.Path.join(uAppData, "ignore-dev-edition-profile"); @@ -185,6 +188,16 @@ var gMainPane = { Cu.reportError("Failed to toggle separate profile mode: " + error); } } + function createOrRemoveSpecialDevEditionFile(onSuccess) { + let uAppData = OS.Constants.Path.userApplicationDataDir; + let ignoreSeparateProfile = OS.Path.join(uAppData, "ignore-dev-edition-profile"); + + if (separateProfileModeCheckbox.checked) { + OS.File.remove(ignoreSeparateProfile).then(onSuccess, revertCheckbox); + } else { + OS.File.writeAtomic(ignoreSeparateProfile, new Uint8Array()).then(onSuccess, revertCheckbox); + } + } const Cc = Components.classes, Ci = Components.interfaces; let separateProfileModeCheckbox = document.getElementById("separateProfileMode"); @@ -194,30 +207,43 @@ var gMainPane = { "featureEnableRequiresRestart" : "featureDisableRequiresRestart", [brandName]); let title = bundle.getFormattedString("shouldRestartTitle", [brandName]); - let shouldProceed = Services.prompt.confirm(window, title, msg) - if (shouldProceed) { - let cancelQuit = Cc["@mozilla.org/supports-PRBool;1"] - .createInstance(Ci.nsISupportsPRBool); - Services.obs.notifyObservers(cancelQuit, "quit-application-requested", - "restart"); - shouldProceed = !cancelQuit.data; + let check = {value: false}; + let prompts = Services.prompt; + let flags = prompts.BUTTON_POS_0 * prompts.BUTTON_TITLE_IS_STRING + + prompts.BUTTON_POS_1 * prompts.BUTTON_TITLE_CANCEL + + prompts.BUTTON_POS_2 * prompts.BUTTON_TITLE_IS_STRING; + let button0Title = bundle.getString("restartNowButton"); + let button2Title = bundle.getString("restartLaterButton"); + let button_index = prompts.confirmEx(window, title, msg, flags, + button0Title, null, button2Title, null, check) + let RESTART_NOW_BUTTON_INDEX = 0; + let CANCEL_BUTTON_INDEX = 1; + let RESTART_LATER_BUTTON_INDEX = 2; - if (shouldProceed) { - Cu.import("resource://gre/modules/osfile.jsm"); - let uAppData = OS.Constants.Path.userApplicationDataDir; - let ignoreSeparateProfile = OS.Path.join(uAppData, "ignore-dev-edition-profile"); - - if (separateProfileModeCheckbox.checked) { - OS.File.remove(ignoreSeparateProfile).then(quitApp, revertCheckbox); - } else { - OS.File.writeAtomic(ignoreSeparateProfile, new Uint8Array()).then(quitApp, revertCheckbox); - } + switch (button_index) { + case CANCEL_BUTTON_INDEX: + revertCheckbox(); return; - } - } + case RESTART_NOW_BUTTON_INDEX: + let shouldProceed = false; + let cancelQuit = Cc["@mozilla.org/supports-PRBool;1"] + .createInstance(Ci.nsISupportsPRBool); + Services.obs.notifyObservers(cancelQuit, "quit-application-requested", + "restart"); + shouldProceed = !cancelQuit.data; - // Revert the checkbox in case we didn't quit - revertCheckbox(); + if (shouldProceed) { + createOrRemoveSpecialDevEditionFile(quitApp); + return; + } + + // Revert the checkbox in case we didn't quit + revertCheckbox(); + return; + case RESTART_LATER_BUTTON_INDEX: + createOrRemoveSpecialDevEditionFile(); + return; + } }, onGetStarted: function (aEvent) { diff --git a/browser/locales/en-US/chrome/browser/preferences/preferences.properties b/browser/locales/en-US/chrome/browser/preferences/preferences.properties index 5d066b0fcbe0..b8be7be8e4b3 100644 --- a/browser/locales/en-US/chrome/browser/preferences/preferences.properties +++ b/browser/locales/en-US/chrome/browser/preferences/preferences.properties @@ -171,6 +171,9 @@ featureEnableRequiresRestart=%S must restart to enable this feature. featureDisableRequiresRestart=%S must restart to disable this feature. shouldRestartTitle=Restart %S +restartNow=Restart Now +restartLater=Restart Later + #### e10S # LOCALIZATION NOTE (e10sFeedbackAfterRestart): This message appears when the user # unchecks "Enable multi-process" on the "General" preferences tab. From 4032e5c5d0d480066cd1da58b0de367e8573d69f Mon Sep 17 00:00:00 2001 From: sakshi Date: Wed, 3 Feb 2016 06:07:00 +0100 Subject: [PATCH 07/34] Bug 1213094 - Removed services.sync.enabled preference. r=markh --- .../components/preferences/in-content/sync.js | 13 ------------ .../preferences/in-content/sync.xul | 3 +-- services/sync/modules/service.js | 21 ++----------------- services/sync/services-sync.js | 5 ----- .../sync/tests/unit/test_service_login.js | 2 +- 5 files changed, 4 insertions(+), 40 deletions(-) diff --git a/browser/components/preferences/in-content/sync.js b/browser/components/preferences/in-content/sync.js index 8d58f11d0346..0fab01450b4b 100644 --- a/browser/components/preferences/in-content/sync.js +++ b/browser/components/preferences/in-content/sync.js @@ -425,19 +425,6 @@ var gSyncPane = { } }, - // Called whenever one of the sync engine preferences is changed. - onPreferenceChanged: function() { - let prefElts = document.querySelectorAll("#syncEnginePrefs > preference"); - let syncEnabled = false; - for (let elt of prefElts) { - if (elt.name.startsWith("services.sync.") && elt.value) { - syncEnabled = true; - break; - } - } - Services.prefs.setBoolPref("services.sync.enabled", syncEnabled); - }, - startOver: function (showDialog) { if (showDialog) { let flags = Services.prompt.BUTTON_POS_0 * Services.prompt.BUTTON_TITLE_IS_STRING + diff --git a/browser/components/preferences/in-content/sync.xul b/browser/components/preferences/in-content/sync.xul index f06ab96f4611..4101cab87834 100644 --- a/browser/components/preferences/in-content/sync.xul +++ b/browser/components/preferences/in-content/sync.xul @@ -4,8 +4,7 @@ -