Do you think you can help with this? It would be really nice to have 1440886 landed.

/js/src/vtune/ is a third-party folder. Could you whitelist this folder?

(In reply to Masatoshi Kimura [:emk] from comment #2) > /js/src/vtune/ is a third-party folder. Could you whitelist this folder? I've opened 1448382 for this. It should land in m-c shortly.

Andi: if you write "bug 1448382" (with "bug" in front of the number) then Bugzilla will auto-link to that bug. It's nice!

Comment on attachment 8962124 [details] Bug 1445601 - Stop using LoadLibraryA in replace_malloc. glandium is a better reviewer for this one.

Comment on attachment 8962125 [details] Bug 1445601 - Stop using LoadLibraryA in GMP. https://reviewboard.mozilla.org/r/230976/#review236838 ::: commit-message-3beac:1 (Diff revision 1) > +Bug 1445601 - Stop using LoadLibraryA in GMP. r=cpearce Please explain in the commit message *why* this change needs to be made. By itself, there's no information directly in the bug (which you didn't file, admitedly) nor in any other patch here, as to why you need to make this change. ::: dom/media/gmp/GMPChild.cpp:293 (Diff revision 1) > + > +template <size_t N> > +constexpr bool IsASCII(const char *const (&aList)[N]) > +{ > + for (size_t i = 0; i < N; ++i) { > + if (!IsASCII(aList[i])) { We have several implementations of this function in our tree already, I think you're better off using one of the existing implementation than rolling your own. ::: dom/media/gmp/GMPChild.cpp:294 (Diff revision 1) > +template <size_t N> > +constexpr bool IsASCII(const char *const (&aList)[N]) > +{ > + for (size_t i = 0; i < N; ++i) { > + if (!IsASCII(aList[i])) { > + return false; Your IsASCII(const char\*) variant of IsASCII scans to the end of the string checking if a non-ASCII character is in the string. But this variant also checks each character. So for each character, you then go into the loop in IsASCII(const char\*) which scans to the end of the string. This means your algorithm is O(N^2), but it should be able to be O(N), right? ::: dom/media/gmp/GMPChild.cpp:316 (Diff revision 1) > "evr.dll", // MFGetStrideForBitmapInfoHeader > "mfplat.dll", // MFCreateSample, MFCreateAlignedMemoryBuffer, MFCreateMediaType > "msmpeg2vdec.dll", // H.264 decoder > "psapi.dll", // For GetMappedFileNameW, see bug 1383611 > }; > + static_assert(IsASCII(whitelist), You should just use NS_IsAscii(const char16_t\*) or NS_IsAscii(const char\*) instead of reimplementing your own function. ::: dom/media/gmp/GMPChild.cpp:325 (Diff revision 1) > SplitAt(", ", aLibs, libs); > for (nsCString lib : libs) { > ToLowerCase(lib); > for (const char* whiteListedLib : whitelist) { > if (lib.EqualsASCII(whiteListedLib)) { > - LoadLibraryA(lib.get()); > + LoadLibraryW(NS_ConvertASCIItoUTF16(lib).get()); Why don't you just store the whitelist strings as wchar_t or char16_t strings insted of converting them at runtime?

Comment on attachment 8962124 [details] Bug 1445601 - Stop using LoadLibraryA in replace_malloc. https://reviewboard.mozilla.org/r/230974/#review236866

Comment on attachment 8962125 [details] Bug 1445601 - Stop using LoadLibraryA in GMP. https://reviewboard.mozilla.org/r/230976/#review236838 > Please explain in the commit message *why* this change needs to be made. By itself, there's no information directly in the bug (which you didn't file, admitedly) nor in any other patch here, as to why you need to make this change. Added some comments. > We have several implementations of this function in our tree already, I think you're better off using one of the existing implementation than rolling your own. See below. > Your IsASCII(const char\*) variant of IsASCII scans to the end of the string checking if a non-ASCII character is in the string. > > But this variant also checks each character. So for each character, you then go into the loop in IsASCII(const char\*) which scans to the end of the string. This means your algorithm is O(N^2), but it should be able to be O(N), right? This function takes a list of string (an array of char*), not a single string. I changed this overload to mozilla::AllOf that bug 1449094 will add. > You should just use NS_IsAscii(const char16_t\*) or NS_IsAscii(const char\*) instead of reimplementing your own function. Filed bug 1449082 to make it possible. > Why don't you just store the whitelist strings as wchar_t or char16_t strings insted of converting them at runtime? Changed the whitelist to char16_t strings.

Comment on attachment 8962125 [details] Bug 1445601 - Stop using LoadLibraryA in GMP. https://reviewboard.mozilla.org/r/230976/#review237348 Thanks, that looks good!

I have seen this patch, and it looks good, but it is going to conflict greatly with a huge refactoring of nsWindowsDllInterceptor that I am about to land. I'd prefer to just incorporate your changes into my refactoring patches to make things easier. Does that work for you?

(In reply to Aaron Klotz [:aklotz] from comment #16) > I have seen this patch, and it looks good, but it is going to conflict > greatly with a huge refactoring of nsWindowsDllInterceptor that I am about > to land. > > I'd prefer to just incorporate your changes into my refactoring patches to > make things easier. Does that work for you? Yes, please do!

Comment on attachment 8962123 [details] Bug 1445601 - Stop using LoadLibraryA in nsWindowsDllInterceptor. https://reviewboard.mozilla.org/r/230972/#review241522 I'm marking r- on this because I made these changes to my refactoring patch. :-)

(In reply to Aaron Klotz [:aklotz] from comment #18) > Comment on attachment 8962123 [details] > Bug 1445601 - Stop using LoadLibraryA in nsWindowsDllInterceptor. > > https://reviewboard.mozilla.org/r/230972/#review241522 > > I'm marking r- on this because I made these changes to my refactoring patch. > :-) What bug are we talking about?

Bug 1432653.

I dropped the nsWindowsDllInterceptor part. It will be incorporated into bug 1432653.

Pushed by VYV03354@nifty.ne.jp: https://hg.mozilla.org/integration/autoland/rev/7a1273c1861c Stop using LoadLibraryA in replace_malloc. r=glandium https://hg.mozilla.org/integration/autoland/rev/4d0b4650e438 Stop using LoadLibraryA in GMP. r=cpearce

https://hg.mozilla.org/mozilla-central/rev/7a1273c1861c https://hg.mozilla.org/mozilla-central/rev/4d0b4650e438