jya mentioned that we have inconsistent naming for HLS / Hls in Bug 1350246. We've decided to unify the naming in this bug. Since 'Hls' is used by Google's Exoplayer, we decided to respect to the naming convention from Exoplayer. Therefore, we will unify the naming into 'HLS' in demuxer/ resource wrapper.java and their corresponding cpp implementation.

Hi John, Our goals as comment 0 said, We want to rename the Hls into HLS only focusing on the classes which ares used by the native implementation. Since Exoplayer's nameing conventional is 'Hls', so we leave this convention in our gecko java codes which did not be used by native. Please help to review this changes, thanks.

Comment on attachment 8876719 [details] Bug 1368907 - Part1 - Rename GeckoHlsDemuxerWrapper and HlsDemuxerCallbacks for naming consistency. https://reviewboard.mozilla.org/r/148050/#review152664 ::: commit-message-52721:1 (Diff revision 1) > +Bug 1368907 - Part1 - GeckoHlsDemuxerWrapper to GeckoHLSDemuxerWrapper HLSDelsDemuxerCallbacksacks to HLSDemuxerCallbacks and the its java file name. r?jolin `HLSDelsDemuxerCallbacksacks` is a typo. Suggestion: "rename GeckoHlsDemuxerWrapper for naming consistency." should be sufficient. ::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/GeckoHLSDemuxerWrapper.java:40 (Diff revision 1) > + } > + } > + > + private GeckoHlsPlayer mPlayer = null; > + > + public static class HLSDemuxerCallbacks extends JNIObject Nit: `GeckoHLSDemuxerWrapper.HLSDemuxerCallbacks` seems redundant, just name it `Callbacks`?

Comment on attachment 8876720 [details] Bug 1368907 - Part2 - Rename GeckoHlsSample to GeckoHLSSample. https://reviewboard.mozilla.org/r/148052/#review152668 ::: commit-message-276c8:1 (Diff revision 1) > +Bug 1368907 - Part2 - Rename GeckoHlsSample to GeckoHLSSample and its related callers. r?jolin Nit: just "Rename GeckoHlsSample to GeckoHLSSample." (Callers were not renamed.)

Comment on attachment 8876721 [details] Bug 1368907 - Part3 - Rename GeckoHlsResourceWrapper to GeckoHLSResourceWrapper for naming consistency. https://reviewboard.mozilla.org/r/148054/#review152672 ::: commit-message-0fb98:1 (Diff revision 1) > +Bug 1368907 - Part3 - Rename GeckoHlsResourceWrapper to GeckoHLSResourceWrapper, HLlsesourceCallbacksH HLSResourceCallbacks and its java file name. r?jolin typo? `HLlsesourceCallbacksH` And I think "Rename GeckoHlsResourceWrapper to GeckoHLSResourceWrapper for naming consistency." should be enough. ::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/GeckoHLSResourceWrapper.java:18 (Diff revision 1) > + private static final String LOGTAG = "GeckoHLSResourceWrapper"; > + private static final boolean DEBUG = false; > + private GeckoHlsPlayer mPlayer = null; > + private boolean mDestroy = false; > + > + public static class HLSResourceCallbacks extends JNIObject Nit: just `Callbacks`

Comment on attachment 8876722 [details] Bug 1368907 - Part4 - Update the moz.build after renaming the files. https://reviewboard.mozilla.org/r/148056/#review152676

Comment on attachment 8876723 [details] Bug 1368907 - Part5 - Regenerate the JNI glue codes after renaming the java files. https://reviewboard.mozilla.org/r/148058/#review152678

Comment on attachment 8876724 [details] Bug 1368907 - Part6 - Modify the corresponding native code callers. https://reviewboard.mozilla.org/r/148060/#review152680

Thanks for the quick review, addressed all the comments. Wait for bug 1350246 to be landed.

Pushed by jacheng@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b7170473b1fb Part1 - Rename GeckoHlsDemuxerWrapper and HlsDemuxerCallbacks for naming consistency. r=jolin https://hg.mozilla.org/integration/autoland/rev/67d66a88e833 Part2 - Rename GeckoHlsSample to GeckoHLSSample. r=jolin https://hg.mozilla.org/integration/autoland/rev/a4e8de872bd0 Part3 - Rename GeckoHlsResourceWrapper to GeckoHLSResourceWrapper for naming consistency. r=jolin https://hg.mozilla.org/integration/autoland/rev/7562c34ed064 Part4 - Update the moz.build after renaming the files. r=jolin https://hg.mozilla.org/integration/autoland/rev/9f8e9e56f886 Part5 - Regenerate the JNI glue codes after renaming the java files. r=jolin https://hg.mozilla.org/integration/autoland/rev/955bb796fd65 Part6 - Modify the corresponding native code callers. r=jolin

https://hg.mozilla.org/mozilla-central/rev/b7170473b1fb https://hg.mozilla.org/mozilla-central/rev/67d66a88e833 https://hg.mozilla.org/mozilla-central/rev/a4e8de872bd0 https://hg.mozilla.org/mozilla-central/rev/7562c34ed064 https://hg.mozilla.org/mozilla-central/rev/9f8e9e56f886 https://hg.mozilla.org/mozilla-central/rev/955bb796fd65

In the future, please do not do rename of files by simply changing the case. It will break things for people using case insensitive file system (like on windows) We have hooks to disable renaming in pushes. However here you didn't rename, you deleted and created a new one (which is even worse as you lose the history)

I apologize that I make the case-insensitive file system bump into some problem. I am not pretty sure the behavior when using HG. I simply use 'git mv' to rename the file but it seems the actual behavior in the patch is 'delete file then create new one'. If I wanna simply change the case of the file name, what should I do? Change file 'A' to file 'B' then change to file 'a'? or just using HG? Thank you for your reminding.

git doesn't have a concept of renaming, only mercurial does. You can trick git log / diff to make it look the file was moved, but that only works if no modifications were made on the file after the rename. Ideally, this shouldn't have been spawned into a new bug, the name of the file should have been changed in the bug the comment was made. as mentioned, renaming with just a case change would normally be blocked from being committed. I'll talk to the mercurial folks so that they add a hook to prevent delete + rename with just case change.