Remove old pinch-zoom paths from Blink

Remove old pinch-zoom paths from Blink

BUG=304869
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=192391

[bokan, 2015-03-20 19:59:55]

bokan@chromium.org changed reviewers:
+ aelias@chromium.org, rbyers@chromium.org

[bokan, 2015-03-20 20:00:15]

Snippety snip snip

[aelias_OOO_until_Jul13, 2015-03-20 20:25:18]

https://codereview.chromium.org/1025543002/diff/60001/Source/core/frame/Pinch...
File Source/core/frame/PinchViewport.cpp (right):

https://codereview.chromium.org/1025543002/diff/60001/Source/core/frame/Pinch...
Source/core/frame/PinchViewport.cpp:239: // TODO(bokan): Seems that in some
tests (svg-resource-fragment-identifier-img-src.html),
I assume this is because you're cleaning up some FIXMEs elsewhere that result in
a behavior change.  I'd suggest making this patch a no-op change assuming the
setting was already enabled to make it more likely to stick, and to make the
cause of any regressions more obvious (instead of having to dig through a
megapatch).

https://codereview.chromium.org/1025543002/diff/60001/Source/core/loader/Fram...
File Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/1025543002/diff/60001/Source/core/loader/Fram...
Source/core/loader/FrameLoader.cpp:1027: // pinch path is enabled on all
platforms for at least one release.
I'd suggest removing the FIXME comment.  Users frequently skip updates on
Android, or they can get a version ten releases behind preinstalled on their
phone, browse a bit on it, and then upgrade to the latest.  For that reason
there's no safe time to drop support for old profile data formats.  There's no
hurry to delete this logic since it's not particularly invasive technical debt.

[bokan, 2015-03-20 23:15:47]

https://codereview.chromium.org/1025543002/diff/60001/Source/core/frame/Pinch...
File Source/core/frame/PinchViewport.cpp (right):

https://codereview.chromium.org/1025543002/diff/60001/Source/core/frame/Pinch...
Source/core/frame/PinchViewport.cpp:239: // TODO(bokan): Seems that in some
tests (svg-resource-fragment-identifier-img-src.html),
On 2015/03/20 20:25:18, aelias wrote:
> I assume this is because you're cleaning up some FIXMEs elsewhere that result
in
> a behavior change.  I'd suggest making this patch a no-op change assuming the
> setting was already enabled to make it more likely to stick, and to make the
> cause of any regressions more obvious (instead of having to dig through a
> megapatch).

Done. (This was actually just moving the FIXME above down here, but that does
have a small observable change. I'll do that in a followup.

https://codereview.chromium.org/1025543002/diff/60001/Source/core/loader/Fram...
File Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/1025543002/diff/60001/Source/core/loader/Fram...
Source/core/loader/FrameLoader.cpp:1027: // pinch path is enabled on all
platforms for at least one release.
On 2015/03/20 20:25:18, aelias wrote:
> I'd suggest removing the FIXME comment.  Users frequently skip updates on
> Android, or they can get a version ten releases behind preinstalled on their
> phone, browse a bit on it, and then upgrade to the latest.  For that reason
> there's no safe time to drop support for old profile data formats.  There's no
> hurry to delete this logic since it's not particularly invasive technical
debt.

Done.

[aelias_OOO_until_Jul13, 2015-03-20 23:31:02]

lgtm!

[Rick Byers, 2015-03-23 19:16:14]

Great!  LGTM with minor nits

https://codereview.chromium.org/1025543002/diff/80001/Source/web/InspectorOve...
File Source/web/InspectorOverlayImpl.cpp (right):

https://codereview.chromium.org/1025543002/diff/80001/Source/web/InspectorOve...
Source/web/InspectorOverlayImpl.cpp:375: // TODO(bokan): |pageScaleFactor| was
previously used to scale from CSS values but is unnecessary
nit: TODO should be FIXME

https://codereview.chromium.org/1025543002/diff/80001/public/web/WebSettings.h
File public/web/WebSettings.h (right):

https://codereview.chromium.org/1025543002/diff/80001/public/web/WebSettings....
public/web/WebSettings.h:193: // TODO(bokan): Temporary, need to remove Chromium
side users.
nit: FIXME.

[bokan, 2015-03-23 19:17:33]

On 2015/03/23 19:16:14, Rick Byers wrote:
> Great!  LGTM with minor nits
> 
>
https://codereview.chromium.org/1025543002/diff/80001/Source/web/InspectorOve...
> File Source/web/InspectorOverlayImpl.cpp (right):
> 
>
https://codereview.chromium.org/1025543002/diff/80001/Source/web/InspectorOve...
> Source/web/InspectorOverlayImpl.cpp:375: // TODO(bokan): |pageScaleFactor| was
> previously used to scale from CSS values but is unnecessary
> nit: TODO should be FIXME
> 
> https://codereview.chromium.org/1025543002/diff/80001/public/web/WebSettings.h
> File public/web/WebSettings.h (right):
> 
>
https://codereview.chromium.org/1025543002/diff/80001/public/web/WebSettings....
> public/web/WebSettings.h:193: // TODO(bokan): Temporary, need to remove
Chromium
> side users.
> nit: FIXME.

Blink-dev has recently settled on TODO(name):
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/fS6tZ-qNx9I

[bokan, 2015-03-23 19:54:54]

The CQ bit was checked by bokan@chromium.org

[bokan, 2015-03-23 19:54:55]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/1025543002/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2015-03-23 19:55:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1025543002/100001

[Rick Byers, 2015-03-23 20:16:06]

On 2015/03/23 19:17:33, bokan wrote:
> On 2015/03/23 19:16:14, Rick Byers wrote:
> > Great!  LGTM with minor nits
> > 
> >
>
https://codereview.chromium.org/1025543002/diff/80001/Source/web/InspectorOve...
> > File Source/web/InspectorOverlayImpl.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1025543002/diff/80001/Source/web/InspectorOve...
> > Source/web/InspectorOverlayImpl.cpp:375: // TODO(bokan): |pageScaleFactor|
was
> > previously used to scale from CSS values but is unnecessary
> > nit: TODO should be FIXME
> > 
> >
https://codereview.chromium.org/1025543002/diff/80001/public/web/WebSettings.h
> > File public/web/WebSettings.h (right):
> > 
> >
>
https://codereview.chromium.org/1025543002/diff/80001/public/web/WebSettings....
> > public/web/WebSettings.h:193: // TODO(bokan): Temporary, need to remove
> Chromium
> > side users.
> > nit: FIXME.
> 
> Blink-dev has recently settled on TODO(name):
> https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/fS6tZ-qNx9I

Oh, great to see (this inconsistency always bugged me) - thank you!

[commit-bot: I haz the power, 2015-03-23 23:15:49]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192391