fix code review comments

Author: iskakaushik

shell/common/display.h

@@ -23,7 +23,7 @@ class Display {
  public:
   //------------------------------------------------------------------------------
   /// @brief Construct a new Display object in case where the display id of the
-  /// display is known. In cases where there are more than one displays, every
+  /// display is known. In cases where there is more than one display, every
   /// display is expected to have a display id.
   ///
   Display(DisplayId display_id, double refresh_rate)
@@ -33,7 +33,8 @@ class Display {
   /// @brief Construct a new Display object when there is only a single display.
   /// When there are multiple displays, every display must have a display id.
   ///
-  Display(double refresh_rate) : display_id_({}), refresh_rate_(refresh_rate) {}
+  explicit Display(double refresh_rate)
+      : display_id_({}), refresh_rate_(refresh_rate) {}
 
   ~Display() = default;
 

shell/common/display_manager.cc

@@ -13,7 +13,7 @@ DisplayManager::DisplayManager() = default;
 
 DisplayManager::~DisplayManager() = default;
 
-double DisplayManager::GetMainDisplayRefreshRate() {
+double DisplayManager::GetMainDisplayRefreshRate() const {
   std::scoped_lock lock(displays_mutex_);
   if (displays_.empty()) {
     return kUnknownDisplayRefreshRate;
@@ -24,19 +24,20 @@ double DisplayManager::GetMainDisplayRefreshRate() {
 
 void DisplayManager::HandleDisplayUpdates(DisplayUpdateType update_type,
                                           std::vector<Display> displays) {
-  CheckDisplayConfiguration(displays);
   std::scoped_lock lock(displays_mutex_);
+  CheckDisplayConfiguration(displays);
   switch (update_type) {
     case DisplayUpdateType::kStartup:
       FML_CHECK(displays_.empty());
       displays_ = displays;
       return;
     default:
-      FML_LOG(ERROR) << "Unknown DisplayUpdateType.";
+      FML_CHECK(false) << "Unknown DisplayUpdateType.";
   }
 }
 
-void DisplayManager::CheckDisplayConfiguration(std::vector<Display> displays) {
+void DisplayManager::CheckDisplayConfiguration(
+    std::vector<Display> displays) const {
   FML_CHECK(!displays.empty());
   if (displays.size() > 1) {
     for (auto& display : displays) {

shell/common/display_manager.h

@@ -19,7 +19,7 @@ enum class DisplayUpdateType {
   /// considered active if:
   ///    1. The frame buffer hardware is connected.
   ///    2. The display is drawable, e.g. it isn't being mirrored from another
-  ///    connected display or sleeping.
+  ///       connected display or sleeping.
   kStartup
 };
 
@@ -31,26 +31,27 @@ class DisplayManager {
   ~DisplayManager();
 
   /// Returns the display refresh rate of the main display. In cases where there
-  /// is only one display connected, it will return that. We do not yer support
+  /// is only one display connected, it will return that. We do not yet support
   /// cases where there are multiple displays.
   ///
   /// When there are no registered displays, it returns
   /// `kUnknownDisplayRefreshRate`.
-  double GetMainDisplayRefreshRate();
+  double GetMainDisplayRefreshRate() const;
 
   /// Handles the display updates.
   void HandleDisplayUpdates(DisplayUpdateType update_type,
                             std::vector<Display> displays);
 
  private:
   /// Guards `displays_` vector.
-  std::mutex displays_mutex_;
+  mutable std::mutex displays_mutex_;
   std::vector<Display> displays_;
 
   /// Checks that the provided display configuration is valid. Currently this
-  /// ensures that all the displays have id in case there are multiple. In case
-  /// there is a single display, it is valid for the display to not have an id.
-  void CheckDisplayConfiguration(std::vector<Display> displays);
+  /// ensures that all the displays have an id in the case there are multiple
+  /// displays. In case where there is a single display, it is valid for the
+  /// display to not have an id.
+  void CheckDisplayConfiguration(std::vector<Display> displays) const;
 };
 
 }  // namespace flutter

shell/platform/darwin/ios/framework/Source/vsync_waiter_ios.mm

@@ -83,6 +83,7 @@ - (void)invalidate {
 - (void)dealloc {
   [self invalidate];
 
+  [display_link_ dealloc];
   [super dealloc];
 }
 
@@ -129,6 +130,7 @@ - (void)onDisplayLink:(CADisplayLink*)link {
 - (void)dealloc {
   [display_link_ invalidate];
 
+  [display_link_ dealloc];
   [super dealloc];
 }
 

shell/platform/embedder/embedder.cc

@@ -1962,7 +1962,7 @@ static bool ValidDisplayConfiguration(const FlutterEngineDisplay* displays,
                                       size_t display_count) {
   std::set<FlutterEngineDisplayId> display_ids;
   for (size_t i = 0; i < display_count; i++) {
-    if (displays[i].single_display && (display_count != 1)) {
+    if (displays[i].single_display && display_count != 1) {
       return false;
     }
     display_ids.insert(displays[i].display_id);

shell/platform/embedder/embedder.h

@@ -1009,7 +1009,7 @@ typedef enum {
 ///
 FlutterEngineResult FlutterEngineNotifyDisplayUpdate(
     FLUTTER_API_SYMBOL(FlutterEngine) engine,
-    const FlutterEngineDisplaysUpdateType update_type,
+    FlutterEngineDisplaysUpdateType update_type,
     const FlutterEngineDisplay* displays,
     size_t display_count);