Fixing Firefox 21+ password import

chrome/common/importer/firefox_importer_utils.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/common/importer/firefox_importer_utils.h"
 
 #include <algorithm>
 #include <map>
 #include <string>
 
@@ skipping 88 matching lines @@
   bool ret = false;
   base::FilePath compatibility_file =
       profile_path.AppendASCII("compatibility.ini");
   std::string content;
   base::ReadFileToString(compatibility_file, &content);
   ReplaceSubstringsAfterOffset(&content, 0, "\r\n", "\n");
   std::vector<std::string> lines;
   base::SplitString(content, '\n', &lines);
 
   for (size_t i = 0; i < lines.size(); ++i) {
-    const std::string& line = lines[i];
+    std::string& line = lines[i];

[jeremy, 2014/01/07 12:37:27]

maintain const and work on a copy of the string below in the #ifdef

     if (line.empty() || line[0] == '#' || line[0] == ';')
       continue;
     size_t equal = line.find('=');
     if (equal != std::string::npos) {
       std::string key = line.substr(0, equal);
       if (key == "LastVersion") {
         base::StringToInt(line.substr(equal + 1), version);
         ret = true;
-      } else if (key == "LastAppDir") {
+      } else if (key == "LastPlatformDir") {

[jeremy, 2014/01/07 12:37:27]

Why are we changing this? This code is cross-platform, are we sure changing the
key doesn't break other platforms?

LastAppDir contains the .app path, right?

         // TODO(evanm): If the path in question isn't convertible to
         // UTF-8, what does Firefox do?  If it puts raw bytes in the
         // file, we could go straight from bytes -> filepath;
         // otherwise, we're out of luck here.
+#if defined (OS_MACOSX)

[jeremy, 2014/01/07 12:37:27]

s/defined (/defined(/

+        // Find the "*.app" component of the path and build up from there.
+        // The resulting path will be .../Firefox.app/Contents/MacOS.
+        // We do this because we don't trust LastPlatformDir to always be
+        // this particular path, without any subdirs, and we want to make
+        // our assumption about Firefox's root being in that path explicit.
+        const std::string dot_app(".app");
+        const size_t pos = line.find(dot_app);

[jeremy, 2014/01/07 12:37:27]

Recommend manipulating as a FilePath:
 * Split using GetComponents().
 *  then build up a new FilePath using Append() to iteratively add path
components.

+        if (pos == std::string::npos) {
+          LOG(ERROR) << line << " doesn't look like a valid Firefox "
+                     << "installation path: missing /*.app/ directory.";
+          return false;
+        }
+        // Replace everything after .app with /Contents/MacOS.
+        line.replace(pos + dot_app.length(),
+                     std::string::npos,
+                     "/Contents/MacOS");
+#endif

[jeremy, 2014/01/07 12:37:27]

#endif  // OS_MACOSX

         *app_path = base::FilePath::FromUTF8Unsafe(line.substr(equal + 1));
       }
     }
   }
   return ret;
 }
 
 bool ReadPrefFile(const base::FilePath& path, std::string* content) {
   if (content == NULL)
     return false;
@@ skipping 144 matching lines @@
         }
       }
     }
   }
 
   StringToLowerASCII(&branding_name);
   if (branding_name.find("iceweasel") != std::string::npos)
     return l10n_util::GetStringUTF16(IDS_IMPORT_FROM_ICEWEASEL);
   return l10n_util::GetStringUTF16(IDS_IMPORT_FROM_FIREFOX);
 }