Bug 1461056 - Remove haveAbsolutePath and tweak some error messages. r?dthayer draft
authorMarkus Stange <mstange@themasta.com>
Mon, 14 May 2018 16:03:24 -0400
changeset 795151 b26881b49f93bfe93b36138ecdab47802805112f
parent 795150 ed916a1fb9c2cdb663e06b8432df0266647160a8
child 795152 5ae915e49bbbb88b49c4ff772b203ac2707cb626
push id109874
push userbmo:mstange@themasta.com
push dateTue, 15 May 2018 03:42:32 +0000
reviewersdthayer
bugs1461056
milestone62.0a1
Bug 1461056 - Remove haveAbsolutePath and tweak some error messages. r?dthayer MozReview-Commit-ID: IyQIyNaKBy3
browser/components/extensions/parent/ext-geckoProfiler.js
--- a/browser/components/extensions/parent/ext-geckoProfiler.js
+++ b/browser/components/extensions/parent/ext-geckoProfiler.js
@@ -387,65 +387,67 @@ this.geckoProfiler = class extends Exten
         },
 
         async getSymbols(debugName, breakpadId) {
           if (symbolCache.size === 0) {
             primeSymbolStore(Services.profiler.sharedLibraries);
           }
 
           const cachedLibInfo = symbolCache.get(`${debugName}/${breakpadId}`);
+          if (!cachedLibInfo) {
+            throw new Error(
+              `The library ${debugName} ${breakpadId} is not in the Services.profiler.sharedLibraries list, ` +
+              "so the local path for it is not known and symbols for it can not be obtained. " +
+              "This usually happens if a content process uses a library that's not used in the parent " +
+              "process - Services.profiler.sharedLibraries only knows about libraries in the parent process.");
+          }
+
+          const {path, debugPath, arch} = cachedLibInfo;
+          if (!OS.Path.split(path).absolute) {
+            throw new Error(
+              `Services.profiler.sharedLibraries did not contain an absolute path for the library ${debugName} ${breakpadId}, ` +
+              "so symbols for this library can not be obtained.");
+          }
 
           const symbolRules = Services.prefs.getCharPref(PREF_GET_SYMBOL_RULES, "localBreakpad");
-          const haveAbsolutePath = cachedLibInfo && OS.Path.split(cachedLibInfo.path).absolute;
 
           // We have multiple options for obtaining symbol information for the given
           // binary.
           //  "localBreakpad"  - Use existing symbol dumps stored in the object directory of a local
-          //      Firefox build, generated using `mach buildsymbols` [requires path]
-          //  "nm"             - Use the command line tool `nm` [linux/mac only, requires path]
-          //  "dump_syms.exe"  - Use the tool dump_syms.exe from the object directory [Windows
-          //      only, requires path]
+          //      Firefox build, generated using `mach buildsymbols`
+          //  "nm"             - Use the command line tool `nm` [linux/mac only]
+          //  "dump_syms.exe"  - Use the tool dump_syms.exe from the object directory [Windows only]
           for (const rule of symbolRules.split(",")) {
             try {
               switch (rule) {
                 case "localBreakpad":
-                  if (haveAbsolutePath) {
-                    const {path} = cachedLibInfo;
-                    const filepath = filePathForSymFileInObjDir(path, debugName, breakpadId);
-                    if (filepath) {
-                      // NOTE: here and below, "return await" is used to ensure we catch any
-                      // errors in the promise. A simple return would give the error to the
-                      // caller.
-                      return await parseSym({filepath});
-                    }
+                  const filepath = filePathForSymFileInObjDir(path, debugName, breakpadId);
+                  if (filepath) {
+                    // NOTE: here and below, "return await" is used to ensure we catch any
+                    // errors in the promise. A simple return would give the error to the
+                    // caller.
+                    return await parseSym({filepath});
                   }
                   break;
                 case "nm":
-                  if (haveAbsolutePath) {
-                    const {path, arch} = cachedLibInfo;
-                    return await getSymbolsFromNM(path, arch);
-                  }
-                  break;
+                  return await getSymbolsFromNM(path, arch);
                 case "dump_syms.exe":
-                  if (haveAbsolutePath) {
-                    const {path, debugPath} = cachedLibInfo;
-                    let dumpSymsPath = dumpSymsPathInObjDir(path);
-                    if (!dumpSymsPath && previouslySuccessfulDumpSymsPath) {
-                      // We may be able to dump symbol for system libraries
-                      // (which are outside the object directory, and for
-                      // which dumpSymsPath will be null) using dump_syms.exe.
-                      // If we know that dump_syms.exe exists, try it.
-                      dumpSymsPath = previouslySuccessfulDumpSymsPath;
-                    }
-                    if (dumpSymsPath) {
-                      const result =
-                        await getSymbolsUsingWindowsDumpSyms(dumpSymsPath, debugPath);
-                      previouslySuccessfulDumpSymsPath = dumpSymsPath;
-                      return result;
-                    }
+                  let dumpSymsPath = dumpSymsPathInObjDir(path);
+                  if (!dumpSymsPath && previouslySuccessfulDumpSymsPath) {
+                    // We may be able to dump symbol for system libraries
+                    // (which are outside the object directory, and for
+                    // which dumpSymsPath will be null) using dump_syms.exe.
+                    // If we know that dump_syms.exe exists, try it.
+                    dumpSymsPath = previouslySuccessfulDumpSymsPath;
+                  }
+                  if (dumpSymsPath) {
+                    const result =
+                      await getSymbolsUsingWindowsDumpSyms(dumpSymsPath, debugPath);
+                    previouslySuccessfulDumpSymsPath = dumpSymsPath;
+                    return result;
                   }
                   break;
               }
             } catch (e) {
               // Each of our options can go wrong for a variety of reasons, so on failure
               // we will try the next one.
               // "localBreakpad" will fail if this is not a local build that's running from the object
               // directory or if the user hasn't run `mach buildsymbols` on it.