Code review comment for lp:~agateau/unity-2d/reason-fv

Revision history for this message
Ugo Riboni (uriboni) wrote :

61 + <arg type="s" name="reason">
62 + <dox:d>Reason for ending the visibility forcing. Must match
63 + with a previous call to BeginForceVisible</dox:d>
64 + </arg>

Please make sure to mention that you just need one call to terminate all outstanding requests for FV for that specific reason for that process.
Would be nice to say that in the commit message as well.

145 + m_forceVisibleReasonHash[service].insert(reason);
146 + if (!service.isEmpty()) {
147 + m_dbusWatcher->addWatchedService(service);
148 + }

What would be the reason for service to be emtpy ? If it's indeed possible, please move the hash insert into the if, otherwise if it's not expected to happen I suggest you put an assert or a critical error message.

163 + if (m_forceVisibleReasonHash.contains(service)) {
164 + m_forceVisibleReasonHash[service].remove(reason);
165 + if (m_forceVisibleReasonHash[service].isEmpty()) {
166 + m_forceVisibleReasonHash.remove(service);

Why do you check if m_forceVisibleReasonHash[service] is emtpy instead of checking if service is empty like you do in the beginFV method ?
This way you're actually checking if the reason is empty, which i don't think is what you want.

review: Needs Fixing

« Back to merge proposal