Mir

Code review comment for lp:~robertcarr/mir/improve-input-acceptance

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I don't think you've addressed my comment above. For example:

928 + if (client_lifecycles[surface_name] == appeared)
929 + {
930 + return;
931 + }
932 + else if (client_lifecycles[surface_name] == vanished)
933 + {
934 + BOOST_THROW_EXCEPTION(std::runtime_error("Waiting for a client (" + surface_name + ") to appear but it has already vanished"));
935 + }
936 + else
937 + {
938 + std::chrono::time_point<std::chrono::system_clock> current_time;
939 + while ((current_time = std::chrono::system_clock::now()) < end_time)
940 + {
941 + lifecycle_condition.wait_for(lg, end_time - current_time);
942 +
943 + // If the surface we are waiting for has appeared, we can return
944 + // otherwise an unrelated lifecycle event has occured and we go back to waiting.
945 + if (client_lifecycles[surface_name] == appeared)
946 + {
947 + return;
948 + }
949 + }
950 + BOOST_THROW_EXCEPTION(std::runtime_error("Timed out waiting for client (" + surface_name + ") to appear"));
951 + }
952 +}

Is more idiomatic and simpler as:

    if (client_lifecycles[surface_name] == vanished)
    {
        BOOST_THROW_EXCEPTION(std::runtime_error("Waiting for a client (" + surface_name + ") to appear but it has already vanished"));
    }

    while (client_lifecycles[surface_name] != appeared)
    {
        if (cv.wait_until(lock, end_time) == st::cv_status::timeout)
            BOOST_THROW_EXCEPTION(std::runtime_error("Timed out waiting for client (" + surface_name + ") to appear"));
    }

the vanished function can also benefit from "wait_until()"

review: Needs Fixing

« Back to merge proposal