New review. I do think we're getting closer but there are still quite a lot of things - sorry. To avoid confusion, I've started numbering from 10 so as not to reuse the same numbers, but this obviously supercedes the old review.
10) You left in the public form fields $mform->cs and ->showavailability although I think you already changed the code to not use them
11) As noted above, sorry but please continue to duplicate the lang strings instead of using '$string['gradeconditionsection'] = $string['gradecondition'];' logic.
12) (Same as last time) Language string:
+$string['groupingnoaccess'] = 'You have to become a member of the group which has access to this section.';
A better way of writing this would probably be 'You do not currently belong to a group which has access to this section'. The current phrasing might be problematic because it suggests people can 'become' a member of that group - depending on how the teacher is running the course, it may or may not be possible to 'become' a member, so it might be misleading.
13) You lost the <?php from the start of conditionlib.php?
14) The reduced duplication in conditionlib makes this much much much better than previous change but I really think it ought to be turned into a set of three classes (condition_info_base, condition_info, and condition_info_section) as per my original comment 9, and remove the 'objtype' field. Basically, classes are supposed to define the type of the object, so having a variable called objtype is not a good sign.
As far as I can see the code you've already written should make this a pretty easy change to carry out: any function that doesn't refer to the objtype variable goes into the base class, anything which only uses it for a table name prefix you can also do in base class (if you have a $tableprefix variable, see my original comment) while other functions might need to be abstract and implemented in the two classes. You could search replace $cmorcs with $item.
15) Regarding the secinfo code. This is good but I think it needs a slightly different approach. It's a difficult area and I had a discussion with Petr (skodak) about it because I wasn't sure.
Basically I see a few problems with the current code:
a) It seems like lots of places that need to call rebuild_course_cache will also need to call rebuild_secinfo, including in existing code (custom modules etc).
b) Unlike rebuild_course_cache there isn't a lazy/deferred rebuild option (where it just sets the value to null and lets it sort it out later).
c) The modinfo code was moved into a nice separate class, OOP-style, in modinfolib.php. But this code is still in functions which seems like a step backward.
d) The static caching has been removed from get_all_sections (agree this cache is not needed if using secinfo cache) but there is no indication for existing callers that this might be a bad function to use now.
e) I'm not sure the availability value/uservisible for sections actually works correctly from a security perspective. (I.e. supposing your section is not available until 16 June - it is currently 1 June - the section contains a forum - a student guesses the URL to that forum, which is easy to do on Moodle - the student should be refused access, but does this happen or do they get in?)
Basically my suggestion is to integrate this code with the existing modinfo cache. It is all 'really' the same thing, i.e. a course cache, and the function of preventing access is also related. I think after this changes it should also improve backward compatibility meaning that fewer changes are needed in unrelated files (like the mod/forum one at least). This would involve:
i) Remove rebuild_course_secinfo, and make rebuild_course_cache do that work (but also including the existing rebuild_course_cache capability to just set the value to null if requested). So if you rebuild course cache, that includes the section cache, and works in the same way.
ii) Remove get_all_sections_secinfo, and add the equivalent to course_modinfo class in modinfolib.php. That would include code in the constructor to make sure it is set and call rebuild_course_cache if necessary (actually that code basically exists, just need to include secinfo when testing the condition), and a new function course_modinfo::get_all_sections that basically just returns the value. Maybe also a course_modinfo::get_section_by_number convenience function that calls get_all_sections then returns the relevant part of the array.
iii) Put get_all_sections back the way it was (for backward compatibility). This might mean duplicating the code rather than calling this function directly from inside rebuild_course_cache (since we don't want the static cache in that case). Maybe this function (get_all_sections) should be moved into deprecatedlib to warn people that there is now a more performant way to do it?
iv) Modify the function cm_info::obtain_dynamic_data, inside the if statement about availability being enabled, so that it makes the check about section availability too.
v) You can see that 'really' it would be good if we turned these section objects into a proper class and set certain fields for it as well (similar to cm_info class), but I suggest that we do NOT include that improvement in this change as it's already big enough. Anyway, putting the code in the right place makes it easier/clearer to do that later.
16) The code at the start of editsection.php where it does complicated if statements and the rebuild_course_secinfo calls; seems to me this should just call (in your code) get_all_sections_secinfo which already contains this logic. Or in the new code, $modinfo->get_section_by_number($number).
17) Your upgrade script sets the savepoint twice for the same number - probably the two blocks should just be combined into a single block.
18) Trivia but there are a few points where double quotes are used for strings that should be single quotes.