1554200 - nsLocalFile's Clone method is too naive, should reuse more information from the original

Status: NEW

(Core :: XPCOM, defect)

Description

[:Gijs (he/him)]

nsLocalFile is basically the embodiment of the notion that in fact, file IO is kind of complicated, and just manipulating some strings and passing them to native APIs is likely to get you in trouble (to wit, bug 1539881).

In any case, the implementation of ::Clone() is a bit unfortunate:

NS_IMETHODIMP
nsLocalFile::Clone(nsIFile** aFile) {
  // Just copy-construct ourselves
  RefPtr<nsLocalFile> file = new nsLocalFile(*this);
  file.forget(aFile);

  return NS_OK;
}

(this is from nsLocalFileWin, but *Unix is the same.)

Why is that unfortunate? Well, because it forces us to re-run InitWithPath, which is relatively cheap but probably still more expensive than just copying all the members of the existing object across, but more importantly, because it sets mDirty to true, and so anything that calls ResolveAndStat() (which is basically almost all methods of the object) will cause us to re-run that code - and that bit is sort of expensive.

This is particularly unfortunate because all directory service lookups return cloned copies of what you look up, which means we'll be re-running all this code several times over for things like the profile directory and other often-reused directory references.