Skip to content

Commit a553fe0

Browse files
committed
LibWeb: Piggyback on HTML::ImageRequest in HTMLObjectElement
Like the piggybacking in CSS, this is also totally ad-hoc, since there's no spec to follow. The code here is weird and definitely sub-optimal as we do a second load if it turns out the loaded resource is an image, but given that object elements are rarely used nowadays, I doubt we'll even notice. That said, we should of course improve this code as we move forward.
1 parent 680fc3f commit a553fe0

File tree

2 files changed

+47
-29
lines changed

2 files changed

+47
-29
lines changed

Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp

Lines changed: 41 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, Andreas Kling <kling@serenityos.org>
2+
* Copyright (c) 2020-2023, Andreas Kling <kling@serenityos.org>
33
*
44
* SPDX-License-Identifier: BSD-2-Clause
55
*/
@@ -8,8 +8,11 @@
88
#include <LibWeb/CSS/StyleComputer.h>
99
#include <LibWeb/DOM/Document.h>
1010
#include <LibWeb/DOM/Event.h>
11+
#include <LibWeb/HTML/DecodedImageData.h>
1112
#include <LibWeb/HTML/HTMLMediaElement.h>
1213
#include <LibWeb/HTML/HTMLObjectElement.h>
14+
#include <LibWeb/HTML/ImageRequest.h>
15+
#include <LibWeb/HTML/PotentialCORSRequest.h>
1316
#include <LibWeb/Layout/ImageBox.h>
1417
#include <LibWeb/Loader/ResourceLoader.h>
1518
#include <LibWeb/MimeSniff/MimeType.h>
@@ -75,7 +78,7 @@ JS::GCPtr<Layout::Node> HTMLObjectElement::create_layout_node(NonnullRefPtr<CSS:
7578
// FIXME: Actually paint the nested browsing context's document, similar to how iframes are painted with FrameBox and NestedBrowsingContextPaintable.
7679
return nullptr;
7780
case Representation::Image:
78-
if (m_image_loader.has_value() && m_image_loader->has_image())
81+
if (image_data())
7982
return heap().allocate_without_realm<Layout::ImageBox>(document(), *this, move(style), *this);
8083
break;
8184
default:
@@ -272,7 +275,7 @@ void HTMLObjectElement::run_object_representation_handler_steps(Optional<Depreca
272275
if (!resource()->has_encoded_data())
273276
return run_object_representation_fallback_steps();
274277

275-
convert_resource_to_image();
278+
load_image();
276279
}
277280

278281
// * Otherwise
@@ -310,22 +313,27 @@ void HTMLObjectElement::run_object_representation_fallback_steps()
310313
update_layout_and_child_objects(Representation::Children);
311314
}
312315

313-
void HTMLObjectElement::convert_resource_to_image()
316+
void HTMLObjectElement::load_image()
314317
{
315-
// FIXME: This is a bit awkward. We convert the Resource to an ImageResource here because we do not know
316-
// until now that the resource is an image. ImageLoader then becomes responsible for handling
317-
// encoding failures, animations, etc. It would be clearer if those features were split from
318-
// ImageLoader into a purpose build class to be shared between here and ImageBox.
319-
m_image_loader.emplace(*this);
320-
321-
m_image_loader->on_load = [this] {
322-
run_object_representation_completed_steps(Representation::Image);
323-
};
324-
m_image_loader->on_fail = [this] {
325-
run_object_representation_fallback_steps();
326-
};
318+
// NOTE: This currently reloads the image instead of reusing the resource we've already downloaded.
319+
auto data = attribute(HTML::AttributeNames::data);
320+
auto url = document().parse_url(data);
321+
m_image_request = HTML::ImageRequest::get_shareable_or_create(*document().page(), url).release_value_but_fixme_should_propagate_errors();
322+
m_image_request->add_callbacks(
323+
[this] {
324+
run_object_representation_completed_steps(Representation::Image);
325+
},
326+
[this] {
327+
run_object_representation_fallback_steps();
328+
});
329+
330+
// If the image request is already available or fetching, no need to start another fetch.
331+
if (m_image_request->is_available() || m_image_request->fetch_controller())
332+
return;
327333

328-
m_image_loader->adopt_object_resource({}, *resource());
334+
auto request = HTML::create_potential_CORS_request(vm(), url, Fetch::Infrastructure::Request::Destination::Image, HTML::CORSSettingAttribute::NoCORS);
335+
request->set_client(&document().relevant_settings_object());
336+
m_image_request->fetch_image(realm(), request);
329337
}
330338

331339
void HTMLObjectElement::update_layout_and_child_objects(Representation representation)
@@ -350,31 +358,38 @@ i32 HTMLObjectElement::default_tab_index_value() const
350358
return 0;
351359
}
352360

361+
RefPtr<DecodedImageData const> HTMLObjectElement::image_data() const
362+
{
363+
if (!m_image_request)
364+
return nullptr;
365+
return m_image_request->image_data();
366+
}
367+
353368
Optional<CSSPixels> HTMLObjectElement::intrinsic_width() const
354369
{
355-
if (m_image_loader.has_value())
356-
return m_image_loader->bitmap(0)->width();
370+
if (auto image_data = this->image_data())
371+
return image_data->intrinsic_width();
357372
return {};
358373
}
359374

360375
Optional<CSSPixels> HTMLObjectElement::intrinsic_height() const
361376
{
362-
if (m_image_loader.has_value())
363-
return m_image_loader->bitmap(0)->height();
377+
if (auto image_data = this->image_data())
378+
return image_data->intrinsic_height();
364379
return {};
365380
}
366381

367382
Optional<float> HTMLObjectElement::intrinsic_aspect_ratio() const
368383
{
369-
if (m_image_loader.has_value())
370-
return static_cast<float>(m_image_loader->bitmap(0)->width()) / static_cast<float>(m_image_loader->bitmap(0)->height());
384+
if (auto image_data = this->image_data())
385+
return image_data->intrinsic_aspect_ratio();
371386
return {};
372387
}
373388

374-
RefPtr<Gfx::Bitmap const> HTMLObjectElement::current_image_bitmap(Gfx::IntSize) const
389+
RefPtr<Gfx::Bitmap const> HTMLObjectElement::current_image_bitmap(Gfx::IntSize size) const
375390
{
376-
if (m_image_loader.has_value())
377-
return m_image_loader->bitmap(m_image_loader->current_frame_index());
391+
if (auto image_data = this->image_data())
392+
return image_data->bitmap(0, size);
378393
return nullptr;
379394
}
380395

Userland/Libraries/LibWeb/HTML/HTMLObjectElement.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
#include <LibWeb/HTML/HTMLElement.h>
1313
#include <LibWeb/HTML/NavigableContainer.h>
1414
#include <LibWeb/Layout/ImageProvider.h>
15-
#include <LibWeb/Loader/ImageLoader.h>
15+
#include <LibWeb/Loader/Resource.h>
1616

1717
namespace Web::HTML {
1818

@@ -59,7 +59,7 @@ class HTMLObjectElement final
5959
void run_object_representation_completed_steps(Representation);
6060
void run_object_representation_fallback_steps();
6161

62-
void convert_resource_to_image();
62+
void load_image();
6363
void update_layout_and_child_objects(Representation);
6464

6565
// ^ResourceClient
@@ -77,7 +77,10 @@ class HTMLObjectElement final
7777
virtual void set_visible_in_viewport(bool) override;
7878

7979
Representation m_representation { Representation::Unknown };
80-
Optional<ImageLoader> m_image_loader;
80+
81+
RefPtr<DecodedImageData const> image_data() const;
82+
83+
RefPtr<ImageRequest> m_image_request;
8184
};
8285

8386
}

0 commit comments

Comments
 (0)