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

Issue 457013002: Avoid absoluteImageURL() creating data URLs for canvas by default. (Closed)

Created:
6 years, 4 months ago by aelias_OOO_until_Jul13
Modified:
6 years, 4 months ago
Reviewers:
Justin Novosad
CC:
blink-reviews, blink-reviews-rendering, eae+blinkwatch, jchaffraix+rendering, zino, leviw+renderwatch, pdr., rune+blink, zoltan1, jamesr
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Avoid absoluteImageURL() creating data URLs for canvas by default. This innocuous-looking call was changed in https://codereview.chromium.org/255563004 to produce huge data URLs for canvas elements. This caused a performance regression in Android WebView which needs to report URLs of the last tapped element to the embedding app. Switch absoluteImageURL() back to its previous behavior of ignoring canvas elements, and introduce a new method absoluteImageURLIncludingCanvasDataURL() that's only used in WebViewImpl::saveImageAt and copyImageAt. NOTRY=true BUG=395859 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179888

Patch Set 1 #

Patch Set 2 : Fix failing copyImageAt unit test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -3 lines) Patch
M Source/core/rendering/HitTestResult.h View 2 chunks +4 lines, -0 lines 0 comments Download
M Source/core/rendering/HitTestResult.cpp View 2 chunks +11 lines, -1 line 0 comments Download
M Source/web/WebViewImpl.cpp View 1 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
aelias_OOO_until_Jul13
Hi junov, PTAL.
6 years, 4 months ago (2014-08-09 02:00:24 UTC) #1
Justin Novosad
lgtm +jamesr for Source/web/OWNERS
6 years, 4 months ago (2014-08-09 18:43:19 UTC) #2
aelias_OOO_until_Jul13
On 2014/08/09 18:43:19, junov wrote: > lgtm > > +jamesr for Source/web/OWNERS I'm also a ...
6 years, 4 months ago (2014-08-09 21:18:21 UTC) #3
aelias_OOO_until_Jul13
The CQ bit was checked by aelias@chromium.org
6 years, 4 months ago (2014-08-09 21:20:00 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/aelias@chromium.org/457013002/20001
6 years, 4 months ago (2014-08-09 21:20:22 UTC) #5
commit-bot: I haz the power
6 years, 4 months ago (2014-08-09 21:20:40 UTC) #6
Message was sent while issue was closed.
Change committed as 179888

Powered by Google App Engine
This is Rietveld 408576698