Skip to content

Commit

Permalink
[ios] Update parameters in TakeSnapshot() in WebState
Browse files Browse the repository at this point in the history
This CL updates parameters in TakeSnapshot() from gfx::RectF and
gfx::Image, UI framework implemented in Chromium, to CGRect and
UIImage.

A generated UIImage is converted to gfx::Image at
https://source.chromium.org/chromium/chromium/src/+/main:ios/web/web_state/web_state_impl_realized_web_state.mm;l=827;drc=a56fa7215e6db4381cd9db0ea39e3c0771022a78;bpv=1;bpt=1
and it's back to UIImage again at
https://source.chromium.org/chromium/chromium/src/+/main:ios/chrome/browser/snapshots/model/snapshot_generator.mm;l=95;drc=a56fa7215e6db4381cd9db0ea39e3c0771022a78;bpv=1;bpt=1
which is not necessary.

Change-Id: I185c376e3d920d340618e83395e68357ac19f0fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5151176
Reviewed-by: Federica Germinario <fedegermi@google.com>
Commit-Queue: Asami Doi <asamidoi@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1243970}
  • Loading branch information
d0iasm authored and Chromium LUCI CQ committed Jan 8, 2024
1 parent d55055d commit 15ae7fe
Show file tree
Hide file tree
Showing 10 changed files with 22 additions and 39 deletions.
16 changes: 6 additions & 10 deletions ios/chrome/browser/snapshots/model/snapshot_generator.mm
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
#import "ios/web/public/thread/web_thread.h"
#import "ios/web/public/web_client.h"
#import "ios/web/public/web_state.h"
#import "ui/gfx/geometry/rect_f.h"
#import "ui/gfx/image/image.h"

namespace {

Expand Down Expand Up @@ -133,23 +131,21 @@ - (void)generateWKWebViewSnapshotWithCompletion:(void (^)(UIImage*))completion {

SnapshotInfo snapshotInfo = [self snapshotInfo];
auto wrappedCompletion =
^(__weak SnapshotGenerator* generator, const gfx::Image& image) {
^(__weak SnapshotGenerator* generator, UIImage* image) {
if (!generator) {
completion(nil);
}
UIImage* snapshot = nil;
if (!image.IsEmpty()) {
snapshot = [generator addOverlays:[generator overlays]
baseImage:image.ToUIImage()
frameInWindow:snapshotInfo.snapshotFrameInWindow];
}
UIImage* snapshot =
[generator addOverlays:[generator overlays]
baseImage:image
frameInWindow:snapshotInfo.snapshotFrameInWindow];
if (completion) {
completion(snapshot);
}
};

__weak SnapshotGenerator* weakSelf = self;
_webState->TakeSnapshot(gfx::RectF(snapshotInfo.snapshotFrameInBaseView),
_webState->TakeSnapshot(snapshotInfo.snapshotFrameInBaseView,
base::BindRepeating(wrappedCompletion, weakSelf));
}

Expand Down
8 changes: 3 additions & 5 deletions ios/chrome/test/wpt/cwt_webdriver_app_interface.mm
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@
#import "ios/web/public/test/navigation_test_util.h"
#import "ios/web/public/ui/crw_web_view_proxy.h"
#import "ios/web/public/web_state.h"
#import "ui/gfx/geometry/rect_f.h"
#import "ui/gfx/image/image.h"

using base::test::ios::WaitUntilConditionOrTimeout;

Expand Down Expand Up @@ -266,9 +264,9 @@ + (NSString*)takeSnapshotOfTabWithID:(NSString*)ID {
UIEdgeInsets insets = webState->GetWebViewProxy().contentInset;
CGRect adjustedBounds = UIEdgeInsetsInsetRect(bounds, insets);

webState->TakeSnapshot(gfx::RectF(adjustedBounds),
base::BindRepeating(^(const gfx::Image& image) {
snapshot = image.ToUIImage();
webState->TakeSnapshot(adjustedBounds,
base::BindRepeating(^(UIImage* image) {
snapshot = image;
}));
});

Expand Down
2 changes: 1 addition & 1 deletion ios/web/public/test/fakes/fake_web_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class FakeWebState : public WebState {
bool HasOpener() const override;
void SetHasOpener(bool has_opener) override;
bool CanTakeSnapshot() const override;
void TakeSnapshot(const gfx::RectF& rect, SnapshotCallback callback) override;
void TakeSnapshot(const CGRect rect, SnapshotCallback callback) override;
void CreateFullPagePdf(base::OnceCallback<void(NSData*)> callback) override;
void CloseMediaPresentations() override;

Expand Down
6 changes: 2 additions & 4 deletions ios/web/public/test/fakes/fake_web_state.mm
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#import "ios/web/public/test/fakes/crw_fake_find_interaction.h"
#import "ios/web/session/session_certificate_policy_cache_impl.h"
#import "ios/web/web_state/policy_decision_state_tracker.h"
#import "ui/gfx/image/image.h"

namespace web {

Expand Down Expand Up @@ -494,9 +493,8 @@
return can_take_snapshot_;
}

void FakeWebState::TakeSnapshot(const gfx::RectF& rect,
SnapshotCallback callback) {
std::move(callback).Run(gfx::Image([[UIImage alloc] init]));
void FakeWebState::TakeSnapshot(const CGRect rect, SnapshotCallback callback) {
std::move(callback).Run([[UIImage alloc] init]);
}

void FakeWebState::CreateFullPagePdf(
Expand Down
10 changes: 2 additions & 8 deletions ios/web/public/web_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,6 @@ typedef id<CRWWebViewProxy> CRWWebViewProxyType;
@class UIView;
typedef UIView<CRWScrollableContent> CRWContentView;

namespace gfx {
class Image;
class RectF;
}

namespace web {
namespace proto {
class WebStateStorage;
Expand Down Expand Up @@ -449,7 +444,7 @@ class WebState : public base::SupportsUserData {
virtual void SetHasOpener(bool has_opener) = 0;

// Callback used to handle snapshots. The parameter is the snapshot image.
typedef base::RepeatingCallback<void(const gfx::Image&)> SnapshotCallback;
typedef base::RepeatingCallback<void(UIImage*)> SnapshotCallback;

// Returns whether TakeSnapshot() can be executed. The API may be disabled if
// the WKWebView IPC mechanism is blocked due to an outstanding JavaScript
Expand All @@ -460,8 +455,7 @@ class WebState : public base::SupportsUserData {
// in the coordinate system of the view returned by GetView(). `callback` is
// asynchronously invoked after performing the snapshot. Prior to iOS 11, the
// callback is invoked with a nil snapshot.
virtual void TakeSnapshot(const gfx::RectF& rect,
SnapshotCallback callback) = 0;
virtual void TakeSnapshot(const CGRect rect, SnapshotCallback callback) = 0;

// Creates PDF representation of the web page and invokes the `callback` with
// the NSData of the PDF or nil if a PDF couldn't be generated.
Expand Down
2 changes: 1 addition & 1 deletion ios/web/web_state/web_state_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ class WebStateImpl final : public WebState {
bool HasOpener() const final;
void SetHasOpener(bool has_opener) final;
bool CanTakeSnapshot() const final;
void TakeSnapshot(const gfx::RectF& rect, SnapshotCallback callback) final;
void TakeSnapshot(const CGRect rect, SnapshotCallback callback) final;
void CreateFullPagePdf(base::OnceCallback<void(NSData*)> callback) final;
void CloseMediaPresentations() final;
void AddObserver(WebStateObserver* observer) final;
Expand Down
3 changes: 1 addition & 2 deletions ios/web/web_state/web_state_impl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -742,8 +742,7 @@ void IgnoreOverRealizationCheck() {
return LIKELY(pimpl_) ? pimpl_->CanTakeSnapshot() : false;
}

void WebStateImpl::TakeSnapshot(const gfx::RectF& rect,
SnapshotCallback callback) {
void WebStateImpl::TakeSnapshot(const CGRect rect, SnapshotCallback callback) {
RealizedState()->TakeSnapshot(rect, std::move(callback));
}

Expand Down
2 changes: 1 addition & 1 deletion ios/web/web_state/web_state_impl_realized_web_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ class WebStateImpl::RealizedWebState final : public NavigationManagerDelegate {
bool HasOpener() const;
void SetHasOpener(bool has_opener);
bool CanTakeSnapshot() const;
void TakeSnapshot(const gfx::RectF& rect, SnapshotCallback callback);
void TakeSnapshot(const CGRect rect, SnapshotCallback callback);
void CreateFullPagePdf(base::OnceCallback<void(NSData*)> callback);
void CloseMediaPresentations();
void CloseWebState();
Expand Down
8 changes: 3 additions & 5 deletions ios/web/web_state/web_state_impl_realized_web_state.mm
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@
#import "ios/web/web_state/ui/crw_web_view_navigation_proxy.h"
#import "ios/web/webui/web_ui_ios_controller_factory_registry.h"
#import "ios/web/webui/web_ui_ios_impl.h"
#import "ui/gfx/geometry/rect_f.h"
#import "ui/gfx/image/image.h"
#import "url/gurl.h"
#import "url/url_constants.h"

Expand Down Expand Up @@ -816,15 +814,15 @@ void set_favicon_status(const FaviconStatus& favicon_status) {
return !running_javascript_dialog_;
}

void WebStateImpl::RealizedWebState::TakeSnapshot(const gfx::RectF& rect,
void WebStateImpl::RealizedWebState::TakeSnapshot(const CGRect rect,
SnapshotCallback callback) {
DCHECK(CanTakeSnapshot());
// Move the callback to a __block pointer, which will be in scope as long
// as the callback is retained.
__block SnapshotCallback shared_callback = std::move(callback);
[web_controller_ takeSnapshotWithRect:rect.ToCGRect()
[web_controller_ takeSnapshotWithRect:rect
completion:^(UIImage* snapshot) {
shared_callback.Run(gfx::Image(snapshot));
shared_callback.Run(snapshot);
}];
}

Expand Down
4 changes: 2 additions & 2 deletions ios/web/web_state/web_state_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#import "ios/web/web_state/web_state_impl.h"
#import "net/test/embedded_test_server/default_handlers.h"
#import "net/test/embedded_test_server/embedded_test_server.h"
#import "ui/gfx/geometry/rect_f.h"
#import "ui/gfx/image/image.h"
#import "ui/gfx/image/image_unittest_util.h"

Expand Down Expand Up @@ -159,7 +158,8 @@ void SetUp() override {
CGRect rect = [web_state()->GetView() bounds];
base::test::ios::SpinRunLoopWithMinDelay(base::Seconds(0.2));
web_state()->TakeSnapshot(
gfx::RectF(rect), base::BindRepeating(^(const gfx::Image& snapshot) {
rect, base::BindRepeating(^(UIImage* image) {
const gfx::Image snapshot = gfx::Image(image);
ASSERT_FALSE(snapshot.IsEmpty());
EXPECT_GT(snapshot.Width(), 0);
EXPECT_GT(snapshot.Height(), 0);
Expand Down

0 comments on commit 15ae7fe

Please sign in to comment.