Skip to content

Commit

Permalink
[printing] Use uint32_t for a page number and counts
Browse files Browse the repository at this point in the history
This CL is a follow-up CL of converting
PrintHostMsg_DidGetPrintedPagesCount to mojo [1] and it updates the
type for the page number and counts with uint32_t as it couldn't
have a negative value.

It keeps using int for JNI and dictionary values and uses
'base::checked_cast<int>' to convert uint to int. In order to make
sure that the value is in the range to cast, this change also adds
the maximum limit for the page number and counts and checks the
value before passing it to JNI or dictionary. Given that the previous
type is int, the maximum limit is std::numeric_limits<int>::max().

[1] https://crrev.com/c/2326857

Bug: 1008939
Change-Id: I5c86514be4e7d549f888c9623136078a3b0c88c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2351580
Reviewed-by: Bo <[email protected]>
Reviewed-by: Andrey Kosyakov <[email protected]>
Reviewed-by: Kinuko Yasuda <[email protected]>
Reviewed-by: Lei Zhang <[email protected]>
Commit-Queue: Julie Kim <[email protected]>
Cr-Commit-Position: refs/heads/master@{#807307}
GitOrigin-RevId: fbf3f1850ace4f195f1e32c640c19fe9f32ab8e4
  • Loading branch information
jkim-julie authored and copybara-github committed Sep 16, 2020
1 parent 97dbfc0 commit 8d1536b
Show file tree
Hide file tree
Showing 15 changed files with 61 additions and 59 deletions.
15 changes: 8 additions & 7 deletions blink/public/web/web_local_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -299,13 +299,14 @@ class WebLocalFrame : public WebFrame {
// CSS3 Paged Media ----------------------------------------------------

// Returns the type of @page size styling for the given page.
virtual PageSizeType GetPageSizeType(int page_index) = 0;
virtual PageSizeType GetPageSizeType(uint32_t page_index) = 0;

// Gets the description for the specified page. This includes preferred page
// size and margins in pixels, assuming 96 pixels per inch. The size and
// margins must be initialized to the default values that are used if auto is
// specified.
virtual void GetPageDescription(int page_index, WebPrintPageDescription*) = 0;
virtual void GetPageDescription(uint32_t page_index,
WebPrintPageDescription*) = 0;

// Scripting --------------------------------------------------------------

Expand Down Expand Up @@ -671,18 +672,18 @@ class WebLocalFrame : public WebFrame {
// node is printed (for now only plugins are supported), instead of the entire
// frame.
// Returns the number of pages that can be printed at the given page size.
virtual int PrintBegin(const WebPrintParams&,
const WebNode& constrain_to_node = WebNode()) = 0;
virtual uint32_t PrintBegin(const WebPrintParams&,
const WebNode& constrain_to_node = WebNode()) = 0;

// Returns the page shrinking factor calculated by webkit (usually
// between 1/1.33 and 1/2). Returns 0 if the page number is invalid or
// not in printing mode.
virtual float GetPrintPageShrink(int page) = 0;
virtual float GetPrintPageShrink(uint32_t page) = 0;

// Prints one page, and returns the calculated page shrinking factor
// (usually between 1/1.33 and 1/2). Returns 0 if the page number is
// invalid or not in printing mode.
virtual float PrintPage(int page_to_print, cc::PaintCanvas*) = 0;
virtual float PrintPage(uint32_t page_to_print, cc::PaintCanvas*) = 0;

// Reformats the WebFrame for screen display.
virtual void PrintEnd() = 0;
Expand Down Expand Up @@ -757,7 +758,7 @@ class WebLocalFrame : public WebFrame {
// page-orientation.
virtual WebSize SpoolSizeInPixelsForTesting(
const WebSize& page_size_in_pixels,
int page_count) = 0;
uint32_t page_count) = 0;

// Prints the frame into the canvas, with page boundaries drawn as one pixel
// wide blue lines. This method exists to support web tests.
Expand Down
6 changes: 3 additions & 3 deletions blink/renderer/core/css/page_rule_collector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ static inline bool ComparePageRules(const StyleRulePage* r1,
}

bool PageRuleCollector::IsLeftPage(const ComputedStyle* root_element_style,
int page_index) const {
uint32_t page_index) const {
bool is_first_page_left = false;
DCHECK(root_element_style);
if (!root_element_style->IsLeftToRightDirection())
Expand All @@ -53,14 +53,14 @@ bool PageRuleCollector::IsLeftPage(const ComputedStyle* root_element_style,
return (page_index + (is_first_page_left ? 1 : 0)) % 2;
}

bool PageRuleCollector::IsFirstPage(int page_index) const {
bool PageRuleCollector::IsFirstPage(uint32_t page_index) const {
// FIXME: In case of forced left/right page, page at index 1 (not 0) can be
// the first page.
return (!page_index);
}

PageRuleCollector::PageRuleCollector(const ComputedStyle* root_element_style,
int page_index,
uint32_t page_index,
const AtomicString& page_name,
MatchResult& match_result)
: is_left_page_(IsLeftPage(root_element_style, page_index)),
Expand Down
8 changes: 4 additions & 4 deletions blink/renderer/core/css/page_rule_collector.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class PageRuleCollector {

public:
PageRuleCollector(const ComputedStyle* root_element_style,
int page_index,
uint32_t page_index,
const AtomicString& page_name,
MatchResult&);

Expand All @@ -44,12 +44,12 @@ class PageRuleCollector {

private:
bool IsLeftPage(const ComputedStyle* root_element_style,
int page_index) const;
uint32_t page_index) const;
bool IsRightPage(const ComputedStyle* root_element_style,
int page_index) const {
uint32_t page_index) const {
return !IsLeftPage(root_element_style, page_index);
}
bool IsFirstPage(int page_index) const;
bool IsFirstPage(uint32_t page_index) const;

void MatchPageRulesForList(HeapVector<Member<StyleRulePage>>& matched_rules,
const HeapVector<Member<StyleRulePage>>& rules);
Expand Down
2 changes: 1 addition & 1 deletion blink/renderer/core/css/resolver/style_resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1172,7 +1172,7 @@ scoped_refptr<ComputedStyle> StyleResolver::PseudoStyleForElement(
}

scoped_refptr<const ComputedStyle> StyleResolver::StyleForPage(
int page_index,
uint32_t page_index,
const AtomicString& page_name) {
scoped_refptr<const ComputedStyle> initial_style =
InitialStyleForElement(GetDocument());
Expand Down
2 changes: 1 addition & 1 deletion blink/renderer/core/css/resolver/style_resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class CORE_EXPORT StyleResolver final : public GarbageCollected<StyleResolver> {
const ComputedStyle* layout_parent_style);

scoped_refptr<const ComputedStyle> StyleForPage(
int page_index,
uint32_t page_index,
const AtomicString& page_name);
scoped_refptr<const ComputedStyle> StyleForText(Text*);
scoped_refptr<ComputedStyle> StyleForViewport();
Expand Down
6 changes: 3 additions & 3 deletions blink/renderer/core/dom/document.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2864,7 +2864,7 @@ void Document::ClearFocusedElementTimerFired(TimerBase*) {
focused_element_->blur();
}

scoped_refptr<const ComputedStyle> Document::StyleForPage(int page_index) {
scoped_refptr<const ComputedStyle> Document::StyleForPage(uint32_t page_index) {
UpdateDistributionForUnknownReasons();

AtomicString page_name;
Expand Down Expand Up @@ -2910,12 +2910,12 @@ void Document::EnsurePaintLocationDataValidForNode(
}
}

bool Document::IsPageBoxVisible(int page_index) {
bool Document::IsPageBoxVisible(uint32_t page_index) {
return StyleForPage(page_index)->Visibility() !=
EVisibility::kHidden; // display property doesn't apply to @page.
}

void Document::GetPageDescription(int page_index,
void Document::GetPageDescription(uint32_t page_index,
WebPrintPageDescription* description) {
scoped_refptr<const ComputedStyle> style = StyleForPage(page_index);

Expand Down
6 changes: 3 additions & 3 deletions blink/renderer/core/dom/document.h
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ class CORE_EXPORT Document : public ContainerNode,
void IncLayoutBlockCounter() { ++layout_blocks_counter_; }
void IncLayoutBlockCounterNG() { ++layout_blocks_counter_ng_; }

scoped_refptr<const ComputedStyle> StyleForPage(int page_index);
scoped_refptr<const ComputedStyle> StyleForPage(uint32_t page_index);

// Ensures that location-based data will be valid for a given node.
//
Expand All @@ -611,13 +611,13 @@ class CORE_EXPORT Document : public ContainerNode,
DocumentUpdateReason reason);

// Returns true if page box (margin boxes and page borders) is visible.
bool IsPageBoxVisible(int page_index);
bool IsPageBoxVisible(uint32_t page_index);

// Gets the description for the specified page. This includes preferred page
// size and margins in pixels, assuming 96 pixels per inch. The size and
// margins must be initialized to the default values that are used if auto is
// specified.
void GetPageDescription(int page_index, WebPrintPageDescription*);
void GetPageDescription(uint32_t page_index, WebPrintPageDescription*);

ResourceFetcher* Fetcher() const { return fetcher_.Get(); }

Expand Down
10 changes: 5 additions & 5 deletions blink/renderer/core/exported/web_frame_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8743,8 +8743,8 @@ TEST_F(WebFrameTest, PrintingBasic)
print_params.print_content_area.width = 500;
print_params.print_content_area.height = 500;

int page_count = frame->PrintBegin(print_params);
EXPECT_EQ(1, page_count);
uint32_t page_count = frame->PrintBegin(print_params);
EXPECT_EQ(1u, page_count);
frame->PrintEnd();
}

Expand Down Expand Up @@ -13038,7 +13038,7 @@ static void TestFramePrinting(WebLocalFrameImpl* frame) {
WebSize page_size(500, 500);
print_params.print_content_area.width = page_size.width;
print_params.print_content_area.height = page_size.height;
EXPECT_EQ(1, frame->PrintBegin(print_params, WebNode()));
EXPECT_EQ(1u, frame->PrintBegin(print_params, WebNode()));
PaintRecorder recorder;
frame->PrintPagesForTesting(recorder.beginRecording(IntRect()), page_size,
page_size);
Expand Down Expand Up @@ -13117,7 +13117,7 @@ TEST_F(WebFrameTest, FirstLetterHasDOMNodeIdWhenPrinting) {
print_params.print_content_area.width = page_size.width;
print_params.print_content_area.height = page_size.height;
WebLocalFrameImpl* frame = web_view_helper.LocalMainFrame();
EXPECT_EQ(1, frame->PrintBegin(print_params, WebNode()));
EXPECT_EQ(1u, frame->PrintBegin(print_params, WebNode()));
PaintRecorder recorder;
frame->PrintPagesForTesting(recorder.beginRecording(IntRect()), page_size,
page_size);
Expand Down Expand Up @@ -13365,7 +13365,7 @@ TEST_F(WebFrameSimTest, PageOrientation) {
WebPrintParams print_params;
print_params.print_content_area.width = page_size.width;
print_params.print_content_area.height = page_size.height;
EXPECT_EQ(4, frame->PrintBegin(print_params, WebNode()));
EXPECT_EQ(4u, frame->PrintBegin(print_params, WebNode()));

WebPrintPageDescription description;

Expand Down
24 changes: 11 additions & 13 deletions blink/renderer/core/frame/web_local_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ class ChromePrintContext : public PrintContext {
PrintContext::BeginPrintMode(printed_page_width_, height);
}

virtual float GetPageShrink(int page_number) const {
virtual float GetPageShrink(uint32_t page_number) const {
IntRect page_rect = page_rects_[page_number];
return printed_page_width_ / page_rect.Width();
}
Expand Down Expand Up @@ -488,7 +488,7 @@ class ChromePluginPrintContext final : public ChromePrintContext {

void EndPrintMode() override { plugin_->PrintEnd(); }

float GetPageShrink(int page_number) const override {
float GetPageShrink(uint32_t page_number) const override {
// We don't shrink the page (maybe we should ask the widget ??)
return 1.0;
}
Expand Down Expand Up @@ -1626,8 +1626,8 @@ WebPlugin* WebLocalFrameImpl::GetPluginToPrint(
return plugin_container ? plugin_container->Plugin() : nullptr;
}

int WebLocalFrameImpl::PrintBegin(const WebPrintParams& print_params,
const WebNode& constrain_to_node) {
uint32_t WebLocalFrameImpl::PrintBegin(const WebPrintParams& print_params,
const WebNode& constrain_to_node) {
WebPluginContainerImpl* plugin_container =
GetPluginToPrintHelper(constrain_to_node);
if (plugin_container && plugin_container->SupportsPaginatedPrint()) {
Expand All @@ -1643,18 +1643,16 @@ int WebLocalFrameImpl::PrintBegin(const WebPrintParams& print_params,
print_context_->BeginPrintMode(size.Width(), size.Height());
print_context_->ComputePageRects(size);

return static_cast<int>(print_context_->PageCount());
return print_context_->PageCount();
}

float WebLocalFrameImpl::GetPrintPageShrink(int page) {
float WebLocalFrameImpl::GetPrintPageShrink(uint32_t page) {
DCHECK(print_context_);
DCHECK_GE(page, 0);
return print_context_->GetPageShrink(page);
}

float WebLocalFrameImpl::PrintPage(int page, cc::PaintCanvas* canvas) {
float WebLocalFrameImpl::PrintPage(uint32_t page, cc::PaintCanvas* canvas) {
DCHECK(print_context_);
DCHECK_GE(page, 0);
DCHECK(GetFrame());
DCHECK(GetFrame()->GetDocument());

Expand Down Expand Up @@ -1697,22 +1695,22 @@ bool WebLocalFrameImpl::CapturePaintPreview(const WebRect& bounds,
return success;
}

PageSizeType WebLocalFrameImpl::GetPageSizeType(int page_index) {
PageSizeType WebLocalFrameImpl::GetPageSizeType(uint32_t page_index) {
return GetFrame()->GetDocument()->StyleForPage(page_index)->GetPageSizeType();
}

void WebLocalFrameImpl::GetPageDescription(
int page_index,
uint32_t page_index,
WebPrintPageDescription* description) {
GetFrame()->GetDocument()->GetPageDescription(page_index, description);
}

WebSize WebLocalFrameImpl::SpoolSizeInPixelsForTesting(
const WebSize& page_size_in_pixels,
int page_count) {
uint32_t page_count) {
int spool_width = page_size_in_pixels.width;
int spool_height = 0;
for (int page_index = 0; page_index < page_count; page_index++) {
for (uint32_t page_index = 0; page_index < page_count; page_index++) {
// Make room for the 1px tall page separator.
if (page_index)
spool_height++;
Expand Down
15 changes: 8 additions & 7 deletions blink/renderer/core/frame/web_local_frame_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,9 @@ class CORE_EXPORT WebLocalFrameImpl final
bool had_redirect,
const WebSourceLocation&) override;
void SendOrientationChangeEvent() override;
PageSizeType GetPageSizeType(int page_index) override;
void GetPageDescription(int page_index, WebPrintPageDescription*) override;
PageSizeType GetPageSizeType(uint32_t page_index) override;
void GetPageDescription(uint32_t page_index,
WebPrintPageDescription*) override;
void ExecuteScript(const WebScriptSource&) override;
void ExecuteScriptInIsolatedWorld(int32_t world_id,
const WebScriptSource&) override;
Expand Down Expand Up @@ -279,10 +280,10 @@ class CORE_EXPORT WebLocalFrameImpl final
void DispatchBeforePrintEvent(
base::WeakPtr<WebPrintClient> print_client) override;
WebPlugin* GetPluginToPrint(const WebNode& constrain_to_node) override;
int PrintBegin(const WebPrintParams&,
const WebNode& constrain_to_node) override;
float GetPrintPageShrink(int page) override;
float PrintPage(int page_to_print, cc::PaintCanvas*) override;
uint32_t PrintBegin(const WebPrintParams&,
const WebNode& constrain_to_node) override;
float GetPrintPageShrink(uint32_t page) override;
float PrintPage(uint32_t page_to_print, cc::PaintCanvas*) override;
void PrintEnd() override;
void DispatchAfterPrintEvent() override;
bool GetPrintPresetOptionsForPlugin(const WebNode&,
Expand All @@ -295,7 +296,7 @@ class CORE_EXPORT WebLocalFrameImpl final
bool IsAdSubframe() const override;
void SetIsAdSubframe(blink::mojom::AdFrameType ad_frame_type) override;
WebSize SpoolSizeInPixelsForTesting(const WebSize& page_size_in_pixels,
int page_count) override;
uint32_t page_count) override;
void PrintPagesForTesting(cc::PaintCanvas*,
const WebSize& page_size_in_pixels,
const WebSize& spool_size_in_pixels) override;
Expand Down
6 changes: 3 additions & 3 deletions blink/renderer/core/page/print_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ void PrintContext::OutputLinkedDestinations(GraphicsContext& context,
// static
String PrintContext::PageProperty(LocalFrame* frame,
const char* property_name,
int page_number) {
uint32_t page_number) {
Document* document = frame->GetDocument();
ScopedPrintContext print_context(frame);
// Any non-zero size is OK here. We don't care about actual layout. We just
Expand Down Expand Up @@ -274,12 +274,12 @@ String PrintContext::PageProperty(LocalFrame* frame,
return String("pageProperty() unimplemented for: ") + property_name;
}

bool PrintContext::IsPageBoxVisible(LocalFrame* frame, int page_number) {
bool PrintContext::IsPageBoxVisible(LocalFrame* frame, uint32_t page_number) {
return frame->GetDocument()->IsPageBoxVisible(page_number);
}

String PrintContext::PageSizeAndMarginsInPixels(LocalFrame* frame,
int page_number,
uint32_t page_number,
int width,
int height,
int margin_top,
Expand Down
6 changes: 3 additions & 3 deletions blink/renderer/core/page/print_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ class CORE_EXPORT PrintContext : public GarbageCollected<PrintContext> {
const FloatSize& page_size_in_pixels);
static String PageProperty(LocalFrame*,
const char* property_name,
int page_number);
static bool IsPageBoxVisible(LocalFrame*, int page_number);
uint32_t page_number);
static bool IsPageBoxVisible(LocalFrame*, uint32_t page_number);
static String PageSizeAndMarginsInPixels(LocalFrame*,
int page_number,
uint32_t page_number,
int width,
int height,
int margin_top,
Expand Down
4 changes: 2 additions & 2 deletions blink/renderer/core/testing/internals.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2320,7 +2320,7 @@ int Internals::numberOfPages(float page_width,
}

String Internals::pageProperty(String property_name,
int page_number,
unsigned page_number,
ExceptionState& exception_state) const {
if (!GetFrame()) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidAccessError,
Expand All @@ -2333,7 +2333,7 @@ String Internals::pageProperty(String property_name,
}

String Internals::pageSizeAndMarginsInPixels(
int page_number,
unsigned page_number,
int width,
int height,
int margin_top,
Expand Down
6 changes: 4 additions & 2 deletions blink/renderer/core/testing/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -380,9 +380,11 @@ class Internals final : public ScriptWrappable {
int numberOfPages(float page_width_in_pixels,
float page_height_in_pixels,
ExceptionState&);
String pageProperty(String, int, ExceptionState& = ASSERT_NO_EXCEPTION) const;
String pageProperty(String,
unsigned,
ExceptionState& = ASSERT_NO_EXCEPTION) const;
String pageSizeAndMarginsInPixels(
int,
unsigned,
int,
int,
int,
Expand Down
4 changes: 2 additions & 2 deletions blink/renderer/core/testing/internals.idl
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@
sequence<DOMString> shortcutIconURLs(Document document);
sequence<DOMString> allIconURLs(Document document);
[RaisesException] long numberOfPages(optional double pageWidthInPixels = 800, optional double pageHeightInPixels = 600);
[RaisesException] DOMString pageProperty(DOMString propertyName, long pageNumber);
[RaisesException] DOMString pageSizeAndMarginsInPixels(long pageIndex, long width, long height, long marginTop, long marginRight, long marginBottom, long marginLeft);
[RaisesException] DOMString pageProperty(DOMString propertyName, unsigned long pageNumber);
[RaisesException] DOMString pageSizeAndMarginsInPixels(unsigned long pageIndex, long width, long height, long marginTop, long marginRight, long marginBottom, long marginLeft);

[RaisesException] float pageScaleFactor();
[RaisesException] void setPageScaleFactor(float scaleFactor);
Expand Down

0 comments on commit 8d1536b

Please sign in to comment.