Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(306)

Issue 476463002: Fix for plugin printing regression. (Closed)

Created:
6 years, 4 months ago by Vitaly Buka (NO REVIEWS)
Modified:
6 years, 4 months ago
CC:
blink-reviews
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Fork of https://codereview.chromium.org/469833003/ Cannot print PDFs In r180112, I removed some seemly dead code, but that code actually existed to make some functions on PrintContext virtual. Instead of hacking in virtuals via a subclass in the web layer, we can just make the functions virtual in the base class. I've also added OVERRIDE annotations to catch this sort of error at compile time. These annotations required me to fix the return type of one of these functions to match. Finally, I've added a test, which appears to be the first unit test to exercise this codepath. BUG=403467 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180220

Patch Set 1 #

Patch Set 2 : Wed 08/13/2014 15:15:15.97 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -19 lines) Patch
M Source/core/page/PrintContext.h View 3 chunks +6 lines, -5 lines 0 comments Download
M Source/core/page/PrintContext.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/web/WebLocalFrameImpl.cpp View 1 3 chunks +10 lines, -12 lines 0 comments Download
M Source/web/tests/WebFrameTest.cpp View 2 chunks +17 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
abarth-chromium
LGTM
6 years, 4 months ago (2014-08-13 22:07:43 UTC) #1
Vitaly Buka (NO REVIEWS)
The CQ bit was checked by vitalybuka@chromium.org
6 years, 4 months ago (2014-08-13 22:35:52 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vitalybuka@chromium.org/476463002/20001
6 years, 4 months ago (2014-08-13 22:37:39 UTC) #3
commit-bot: I haz the power
Committed patchset #2 (20001) as 180220
6 years, 4 months ago (2014-08-14 01:46:16 UTC) #4
adamk
On 2014/08/14 at 01:46:16, commit-bot wrote: > Committed patchset #2 (20001) as 180220 The test ...
6 years, 4 months ago (2014-08-14 16:05:25 UTC) #5
adamk
To be clear, the crashes occur only on the Android bot.
6 years, 4 months ago (2014-08-14 16:06:29 UTC) #6
abarth-chromium
6 years, 4 months ago (2014-08-14 16:13:27 UTC) #7
Message was sent while issue was closed.
On 2014/08/14 at 16:06:29, adamk wrote:
> To be clear, the crashes occur only on the Android bot.

Please disable the test on Android.  I'll look in more detail.

Powered by Google App Engine
This is Rietveld 408576698