ronantakizawa/codereview-bench huggingface dataset:
Repository: servo/servo
File: components/shared/embedder/lib.rs (Rust)

Code before:
        range: Range<usize>,
        format: PixelFormat,
    ) -> Self {
        Self {
            width,
            height,
            format,
            data,
            range,
        }
    }

    /// Return the bytes belonging to the first image frame.
    pub fn data(&self) -> &[u8] {
        &self.data[self.range.clone()]
    }
}

#[derive(Clone, Debug, Deserialize, Serialize)]
pub enum LogLevel {
    Log,
    Debug,
    Info,
    Warn,
    Error,
    Clear,
    Trace,
}

impl From<LogLevel> for log::Level {
    fn from(value: LogLevel) -> Self {
        match value {
            LogLevel::Log => log::Level::Info,
            LogLevel::Clear => log::Level::Info,

            LogLevel::Debug => log::Level::Debug,
            LogLevel::Info => log::Level::Info,
            LogLevel::Warn => log::Level::Warn,
            LogLevel::Error => log::Level::Error,
            LogLevel::Trace => log::Level::Trace,
        }
    }
}

/// Messages towards the embedder.
#[derive(Deserialize, IntoStaticStr, Serialize)]
pub enum EmbedderMsg {
    /// A status message to be displayed by the browser chrome.
    Status(WebViewId, Option<String>),
    /// Alerts the embedder that the current page has changed its title.
    ChangePageTitle(WebViewId, Option<String>),

Review comment (suggestion):
As a followup, this variant should be removed from this enum and a separate `Clear` message should be sent to devtools and the API.

Code after:
        range: Range<usize>,
        format: PixelFormat,
    ) -> Self {
        Self {
            width,
            height,
            format,
            data,
            range,
        }
    }

    /// Return the bytes belonging to the first image frame.
    pub fn data(&self) -> &[u8] {
        &self.data[self.range.clone()]
    }
}

#[derive(Clone, Debug, Deserialize, Serialize)]
pub enum ConsoleLogLevel {
    Log,
    Debug,
    Info,
    Warn,
    Error,
    Clear,
    Trace,
}

impl From<ConsoleLogLevel> for log::Level {
    fn from(value: ConsoleLogLevel) -> Self {
        match value {
            ConsoleLogLevel::Log => log::Level::Info,
            ConsoleLogLevel::Clear => log::Level::Info,
            ConsoleLogLevel::Debug => log::Level::Debug,
            ConsoleLogLevel::Info => log::Level::Info,
            ConsoleLogLevel::Warn => log::Level::Warn,
            ConsoleLogLevel::Error => log::Level::Error,
            ConsoleLogLevel::Trace => log::Level::Trace,
        }
    }
}

/// Messages towards the embedder.
#[derive(Deserialize, IntoStaticStr, Serialize)]
pub enum EmbedderMsg {
    /// A status message to be displayed by the browser chrome.
    Status(WebViewId, Option<String>),
    /// Alerts the embedder that the current page has changed its title.
    ChangePageTitle(WebViewId, Option<String>),
    /// Move the window to a point
Repository: servo/servo
File: ports/servoshell/desktop/headless_window.rs (Rust)

Code before:
    fn window_rect(&self) -> DeviceIndependentIntRect {
        convert_rect_to_css_pixel(
            DeviceIntRect::from_origin_and_size(self.window_position.get(), self.inner_size.get()),
            self.hidpi_scale_factor(),
        )
    }

    fn rendering_context(&self) -> Rc<dyn RenderingContext> {
        self.rendering_context.clone()
    }

    fn maximize(&self, webview: &WebView) {
        self.window_position.set(Point2D::zero());
        self.inner_size.set(self.screen_size);
        // Because we are managing the rendering surface ourselves, there will be no other
        // notification (such as from the display manager) that it has changed size, so we
        // must notify the `Paint` here.
        webview.resize(PhysicalSize::new(
            self.screen_size.width as u32,
            self.screen_size.height as u32,
        ));
    }

    fn show_console_message(&self, level: servo::ConsoleLogLevel, message: &str) {
        println!("{}", message);
        log::log!(level.into(), "{}", message);
    }
}


Review comment (suggestion):
I think you can omit this.
```suggestion
        log::log!(level.into(), "{message}");
```

Code after:
    fn window_rect(&self) -> DeviceIndependentIntRect {
        convert_rect_to_css_pixel(
            DeviceIntRect::from_origin_and_size(self.window_position.get(), self.inner_size.get()),
            self.hidpi_scale_factor(),
        )
    }

    fn rendering_context(&self) -> Rc<dyn RenderingContext> {
        self.rendering_context.clone()
    }

    fn maximize(&self, webview: &WebView) {
        self.window_position.set(Point2D::zero());
        self.inner_size.set(self.screen_size);
        // Because we are managing the rendering surface ourselves, there will be no other
        // notification (such as from the display manager) that it has changed size, so we
        // must notify the `Paint` here.
        webview.resize(PhysicalSize::new(
            self.screen_size.width as u32,
            self.screen_size.height as u32,
        ));
    }

    fn show_console_message(&self, level: servo::ConsoleLogLevel, message: &str) {
        println!("{message}");
        log::log!(level.into(), "{message}");
    }
}
Repository: servo/servo
File: components/servo/network_manager.rs (Rust)

Code before:
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at https://mozilla.org/MPL/2.0/. */

use std::collections::HashSet;

use net_traits::ResourceThreads;

#[derive(Clone, Debug, PartialEq)]
pub struct CacheEntry {
    key: String,
}

impl CacheEntry {
    pub fn new(key: String) -> CacheEntry {
        CacheEntry { key }
    }

    pub fn key(&self) -> String {
        self.key.clone()
    }
}

/// Provides APIs for managing network-related state.
///
/// `NetworkManager` is responsible for data owned by the networking layer,
/// such as the HTTP cache. This data is not considered site data and is
/// therefore intentionally separate from `SiteDataManager`.
pub struct NetworkManager {
    public_resource_threads: ResourceThreads,
    private_resource_threads: ResourceThreads,
}

impl NetworkManager {
    pub(crate) fn new(
        public_resource_threads: ResourceThreads,
        private_resource_threads: ResourceThreads,
    ) -> Self {
        Self {
            public_resource_threads,
            private_resource_threads,
        }

Review comment (suggestion):
```suggestion
    pub fn new(key: String) -> Self {
        Self { key }
    }
```

Code after:
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at https://mozilla.org/MPL/2.0/. */

use std::collections::HashSet;

use net_traits::ResourceThreads;

#[derive(Clone, Debug, PartialEq)]
pub struct CacheEntry {
    key: String,
}

impl CacheEntry {
    pub fn new(key: String) -> Self {
        Self { key }
    }

    pub fn key(&self) -> &str {
        &self.key
    }
}

/// Provides APIs for managing network-related state.
///
/// `NetworkManager` is responsible for data owned by the networking layer,
/// such as the HTTP cache. This data is not considered site data and is
/// therefore intentionally separate from `SiteDataManager`.
pub struct NetworkManager {
    public_resource_threads: ResourceThreads,
    private_resource_threads: ResourceThreads,
}

impl NetworkManager {
    pub(crate) fn new(
        public_resource_threads: ResourceThreads,
        private_resource_threads: ResourceThreads,
    ) -> Self {
        Self {
            public_resource_threads,
            private_resource_threads,
        }
Repository: servo/servo
File: components/servo/network_manager.rs (Rust)

Code before:
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at https://mozilla.org/MPL/2.0/. */

use std::collections::HashSet;

use net_traits::ResourceThreads;

#[derive(Clone, Debug, PartialEq)]
pub struct CacheEntry {
    key: String,
}

impl CacheEntry {
    pub fn new(key: String) -> CacheEntry {
        CacheEntry { key }
    }

    pub fn key(&self) -> String {
        self.key.clone()
    }
}

/// Provides APIs for managing network-related state.
///
/// `NetworkManager` is responsible for data owned by the networking layer,
/// such as the HTTP cache. This data is not considered site data and is
/// therefore intentionally separate from `SiteDataManager`.
pub struct NetworkManager {
    public_resource_threads: ResourceThreads,
    private_resource_threads: ResourceThreads,
}

impl NetworkManager {
    pub(crate) fn new(
        public_resource_threads: ResourceThreads,
        private_resource_threads: ResourceThreads,
    ) -> Self {
        Self {
            public_resource_threads,
            private_resource_threads,
        }
    }

    /// Returns cache entries currently stored in the HTTP cache.
    ///

Review comment (suggestion):
I would avoid the clone here:

```suggestion
    pub fn key(&self) -> &str {
        &self.key
    }
```

Code after:
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at https://mozilla.org/MPL/2.0/. */

use std::collections::HashSet;

use net_traits::ResourceThreads;

#[derive(Clone, Debug, PartialEq)]
pub struct CacheEntry {
    key: String,
}

impl CacheEntry {
    pub fn new(key: String) -> Self {
        Self { key }
    }

    pub fn key(&self) -> &str {
        &self.key
    }
}

/// Provides APIs for managing network-related state.
///
/// `NetworkManager` is responsible for data owned by the networking layer,
/// such as the HTTP cache. This data is not considered site data and is
/// therefore intentionally separate from `SiteDataManager`.
pub struct NetworkManager {
    public_resource_threads: ResourceThreads,
    private_resource_threads: ResourceThreads,
}

impl NetworkManager {
    pub(crate) fn new(
        public_resource_threads: ResourceThreads,
        private_resource_threads: ResourceThreads,
    ) -> Self {
        Self {
            public_resource_threads,
            private_resource_threads,
        }
    }

    /// Returns cache entries currently stored in the HTTP cache.
    ///
Repository: servo/servo
File: components/script/dom/subtlecrypto/hmac_operation.rs (Rust)

Code before:
    let length = match normalized_derived_key_algorithm.length {
        // If the length member of normalizedDerivedKeyAlgorithm is not present:
        None => {
            // Let length be the block size in bits of the hash function identified by the hash
            // member of normalizedDerivedKeyAlgorithm.
            hash_fucntion_block_size_in_bits(normalized_derived_key_algorithm.hash.name())?
        },
        // Otherwise, if the length member of normalizedDerivedKeyAlgorithm is non-zero:
        Some(length) if length != 0 => {
            // Let length be equal to the length member of normalizedDerivedKeyAlgorithm.
            length
        },
        // Otherwise:
        _ => {
            // throw a TypeError.
            return Err(Error::Type("[[length]] must not be zero".to_string()));
        },
    };

    // Step 2. Return length.
    Ok(Some(length))
}

/// Return the block size in bits of a hash function, according to Figure 1 of
/// <https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.180-4.pdf>.
fn hash_fucntion_block_size_in_bits(hash: &str) -> Result<u32, Error> {
    match hash {
        ALG_SHA1 => Ok(512),
        ALG_SHA256 => Ok(512),
        ALG_SHA384 => Ok(1024),
        ALG_SHA512 => Ok(1024),
        _ => Err(Error::NotSupported(Some(
            "Unidentified hash member".to_string(),
        ))),
    }
}


Review comment (suggestion):
```suggestion
fn hash_function_block_size_in_bits(hash: &str) -> Result<u32, Error> {
```

Code after:
    let length = match normalized_derived_key_algorithm.length {
        // If the length member of normalizedDerivedKeyAlgorithm is not present:
        None => {
            // Let length be the block size in bits of the hash function identified by the hash
            // member of normalizedDerivedKeyAlgorithm.
            hash_function_block_size_in_bits(normalized_derived_key_algorithm.hash.name())?
        },
        // Otherwise, if the length member of normalizedDerivedKeyAlgorithm is non-zero:
        Some(length) if length != 0 => {
            // Let length be equal to the length member of normalizedDerivedKeyAlgorithm.
            length
        },
        // Otherwise:
        _ => {
            // throw a TypeError.
            return Err(Error::Type("[[length]] must not be zero".to_string()));
        },
    };

    // Step 2. Return length.
    Ok(Some(length))
}

/// Return the block size in bits of a hash function, according to Figure 1 of
/// <https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.180-4.pdf>.
fn hash_function_block_size_in_bits(hash: &str) -> Result<u32, Error> {
    match hash {
        ALG_SHA1 => Ok(512),
        ALG_SHA256 => Ok(512),
        ALG_SHA384 => Ok(1024),
        ALG_SHA512 => Ok(1024),
        _ => Err(Error::NotSupported(Some(
            "Unidentified hash member".to_string(),
        ))),
    }
}
Repository: servo/servo
File: ports/servoshell/window.rs (Rust)

Code before:

    fn show_embedder_control(&self, _: WebViewId, _: EmbedderControl) {}
    fn hide_embedder_control(&self, _: WebViewId, _: EmbedderControlId) {}
    fn dismiss_embedder_controls_for_webview(&self, _: WebViewId) {}
    fn show_bluetooth_device_dialog(
        &self,
        _: WebViewId,
        _devices: Vec<String>,
        _: GenericSender<Option<String>>,
    ) {
    }
    fn show_permission_dialog(&self, _: WebViewId, _: PermissionRequest) {}
    fn show_http_authentication_dialog(&self, _: WebViewId, _: AuthenticationRequest) {}

    fn notify_input_event_handled(
        &self,
        _webview: &WebView,
        _id: InputEventId,
        _result: InputEventResult,
    ) {
    }

    fn notify_media_session_event(&self, _: MediaSessionEvent) {}
    fn notify_crashed(&self, _: WebView, _reason: String, _backtrace: Option<String>) {}
    fn show_console_message(&self, _level: ConsoleLogLevel, _message: &str) {}
}


Review comment (suggestion):
> Hrm. Note that headless windows will never be focused either. This might need to change here as well.

I added a simple API for `PlatformWindow`. We can then handle `HeadlessWindow`/Android/OHOS consistently.

Code after:

    fn show_embedder_control(&self, _: WebViewId, _: EmbedderControl) {}
    fn hide_embedder_control(&self, _: WebViewId, _: EmbedderControlId) {}
    fn dismiss_embedder_controls_for_webview(&self, _: WebViewId) {}
    fn show_bluetooth_device_dialog(
        &self,
        _: WebViewId,
        _devices: Vec<String>,
        _: GenericSender<Option<String>>,
    ) {
    }
    fn show_permission_dialog(&self, _: WebViewId, _: PermissionRequest) {}
    fn show_http_authentication_dialog(&self, _: WebViewId, _: AuthenticationRequest) {}

    fn notify_input_event_handled(
        &self,
        _webview: &WebView,
        _id: InputEventId,
        _result: InputEventResult,
    ) {
    }

    fn notify_media_session_event(&self, _: MediaSessionEvent) {}
    fn notify_crashed(&self, _: WebView, _reason: String, _backtrace: Option<String>) {}
    fn show_console_message(&self, _level: ConsoleLogLevel, _message: &str) {}
    fn has_winit_window(&self) -> bool {
        false
    }
}
Repository: servo/servo
File: components/script/dom/gamepad/gamepadhapticactuator.rs (Rust)

Code before:
                    let message = DOMString::from("complete");
                    promise.resolve_native(&message, CanGc::note());
                })
            );
        }
    }

    /// <https://www.w3.org/TR/gamepad/#handling-visibility-change>
    pub(crate) fn handle_visibility_change(&self) {
        if self.playing_effect_promise.borrow().is_none() {
            return;
        }

        let this = Trusted::new(self);
        self.global().task_manager().gamepad_task_source().queue(
            task!(stop_playing_effect: move || {
                let actuator = this.root();
                let Some(promise) = actuator.playing_effect_promise.borrow_mut().take() else {
                    return;
                };
                let message = DOMString::from("preempted");
                promise.resolve_native(&message, CanGc::note());
            }),
        );

        let callback = GenericCallback::new(|_msg| ()).expect("Could not create callback");

        let document = self.global().as_window().Document();
        let event = EmbedderMsg::StopGamepadHapticEffect(
            document.webview_id(),
            self.gamepad_index as usize,
            callback,
        );
        self.global().as_window().send_to_embedder(event);
    }
}


Review comment (suggestion):
```suggestion
        let callback = GenericCallback::new(|_| ()).expect("Could not create callback");
```

Code after:
                    let message = DOMString::from("complete");
                    promise.resolve_native(&message, CanGc::note());
                })
            );
        }
    }

    /// <https://www.w3.org/TR/gamepad/#handling-visibility-change>
    pub(crate) fn handle_visibility_change(&self) {
        if self.playing_effect_promise.borrow().is_none() {
            return;
        }

        let this = Trusted::new(self);
        self.global().task_manager().gamepad_task_source().queue(
            task!(stop_playing_effect: move || {
                let actuator = this.root();
                let Some(promise) = actuator.playing_effect_promise.borrow_mut().take() else {
                    return;
                };
                let message = DOMString::from("preempted");
                promise.resolve_native(&message, CanGc::note());
            }),
        );

        let callback = GenericCallback::new(|_| ()).expect("Could not create callback");

        let document = self.global().as_window().Document();
        let event = EmbedderMsg::StopGamepadHapticEffect(
            document.webview_id(),
            self.gamepad_index as usize,
            callback,
        );
        self.global().as_window().send_to_embedder(event);
    }
}
Repository: servo/servo
File: ports/servoshell/egl/gamepad.rs (Rust)

Code before:
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at https://mozilla.org/MPL/2.0/. */

use servo::{GamepadHapticEffectType, WebView};

/// A dummy version of [`crate::desktop::GamepadSupport`] used to avoid conditional compilation in
/// servoshell and as a skeleton to implement gamepad support for platforms that do not
/// currently support it.
pub(crate) struct GamepadSupport;

impl GamepadSupport {
    pub(crate) fn maybe_new() -> Option<Self> {
        None
    }

    pub(crate) fn handle_gamepad_events(&mut self, _active_webview: WebView) {
        unreachable!("Dummy gamepad support should never be called.");
    }

    pub(crate) fn play_haptic_effect(
        &mut self,
        _index: usize,
        _effect_type: GamepadHapticEffectType,
        _effect_complete_sender: Box<dyn Fn(bool)>,
    ) {
        unreachable!("Dummy gamepad support should never be called.");
    }

    pub(crate) fn stop_haptic_effect(&mut self, _index: usize) -> bool {
        unreachable!("Dummy gamepad support should never be called.");
    }
}


Review comment (suggestion):
```suggestion
        _effect_complete_callback: Box<dyn Fn(bool)>,
```

Code after:
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at https://mozilla.org/MPL/2.0/. */

use servo::{GamepadHapticEffectType, WebView};

/// A dummy version of [`crate::desktop::GamepadSupport`] used to avoid conditional compilation in
/// servoshell and as a skeleton to implement gamepad support for platforms that do not
/// currently support it.
pub(crate) struct GamepadSupport;

impl GamepadSupport {
    pub(crate) fn maybe_new() -> Option<Self> {
        None
    }

    pub(crate) fn handle_gamepad_events(&mut self, _active_webview: WebView) {
        unreachable!("Dummy gamepad support should never be called.");
    }

    pub(crate) fn play_haptic_effect(
        &mut self,
        _index: usize,
        _effect_type: GamepadHapticEffectType,
        _effect_complete_callback: Box<dyn FnOnce(bool)>,
    ) {
        unreachable!("Dummy gamepad support should never be called.");
    }

    pub(crate) fn stop_haptic_effect(&mut self, _index: usize) -> bool {
        unreachable!("Dummy gamepad support should never be called.");
    }
}
Repository: servo/servo
File: components/servo/network_manager.rs (Rust)

Code before:
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at https://mozilla.org/MPL/2.0/. */

use net_traits::ResourceThreads;

/// Provides APIs for managing network related state.
///
/// `NetworkManager` is responsible for data owned by the networking layer,
/// such as the HTTP cache. This data is not considered site data and is
/// therefore intentionally separate from `SiteDataManager`.
///
/// Additional network related functionality (e.g. protocol handlers, etc.)
/// will be added here over time.
pub struct NetworkManager {
    public_resource_threads: ResourceThreads,
    private_resource_threads: ResourceThreads,
}

impl NetworkManager {
    pub(crate) fn new(
        public_resource_threads: ResourceThreads,
        private_resource_threads: ResourceThreads,
    ) -> Self {
        Self {
            public_resource_threads,
            private_resource_threads,
        }
    }

    /// Clears the network (HTTP) cache.
    ///

Review comment (suggestion):
Super nit:

```suggestion
/// Provides APIs for managing network-related state.
```

Code after:
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at https://mozilla.org/MPL/2.0/. */

use net_traits::ResourceThreads;

/// Provides APIs for managing network-related state.
///
/// `NetworkManager` is responsible for data owned by the networking layer,
/// such as the HTTP cache. This data is not considered site data and is
/// therefore intentionally separate from `SiteDataManager`.
pub struct NetworkManager {
    public_resource_threads: ResourceThreads,
    private_resource_threads: ResourceThreads,
}

impl NetworkManager {
    pub(crate) fn new(
        public_resource_threads: ResourceThreads,
        private_resource_threads: ResourceThreads,
    ) -> Self {
        Self {
            public_resource_threads,
            private_resource_threads,
        }
    }

    /// Clears the network (HTTP) cache.
    ///
    /// This removes all cached network responses maintained by the networking
    /// layer for both public and private browsing contexts.
    ///