Deprecate currentView, useCurrentView properties of SVGSVGElement and SVGViewSpec interface

Deprecate currentView, useCurrentView properties of SVGSVGElement and SVGViewSpec interface

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Dfqld7wjHJM

BUG=629445
Committed: https://crrev.com/11cc1db033d902c13c429f8bee0a13b6381c8e4e
Cr-Commit-Position: refs/heads/master@{#408958}

[Shanmuga Pandi, 2016-07-21 12:54:21]

Description was changed from

==========
Remove currentView, useCurrentView properties of SVGSVGElement and SVGViewSpec
interface

BUG=629445
==========

to

==========
Remove currentView, useCurrentView properties of SVGSVGElement and SVGViewSpec
interface

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Dfqld7wjHJM

BUG=629445
==========

[Shanmuga Pandi, 2016-07-21 12:54:21]

shanmuga.m@samsung.com changed reviewers:
+ rob.buis@samsung.com

[Shanmuga Pandi, 2016-07-21 12:54:40]

On 2016/07/21 12:54:21, Shanmuga Pandi wrote:
> mailto:shanmuga.m@samsung.com changed reviewers:
> + mailto:rob.buis@samsung.com

PTAL!!
Thanks

[fs, 2016-07-21 13:08:27]

fs@opera.com changed reviewers:
+ fs@opera.com

[fs, 2016-07-21 13:08:27]

https://codereview.chromium.org/2163213007/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGSVGElement.cpp (left):

https://codereview.chromium.org/2163213007/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGSVGElement.cpp:661: if
(fragmentIdentifier.startsWith("svgView(")) {
This is still in the spec AFAIK. the only thing that should be removed here is:

 * the SVGViewSpec interface
 * the SVGSVGElement.useCurrentView and SVGSVGElement.currentView

i.e the DOM exposed bits. Any cleanup that isn't obvious "JS only" (like
removing the ScriptWrappable and related bits; *FromJavascript methods;
obviously dead code) is best done as a follow-up (and the removal here would not
be subject to that.)

[Shanmuga Pandi, 2016-07-21 13:11:18]

On 2016/07/21 13:08:27, fs wrote:
>
https://codereview.chromium.org/2163213007/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/svg/SVGSVGElement.cpp (left):
> 
>
https://codereview.chromium.org/2163213007/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/svg/SVGSVGElement.cpp:661: if
> (fragmentIdentifier.startsWith("svgView(")) {
> This is still in the spec AFAIK. the only thing that should be removed here
is:
> 
>  * the SVGViewSpec interface
>  * the SVGSVGElement.useCurrentView and SVGSVGElement.currentView
> 
> i.e the DOM exposed bits. Any cleanup that isn't obvious "JS only" (like
> removing the ScriptWrappable and related bits; *FromJavascript methods;
> obviously dead code) is best done as a follow-up (and the removal here would
not
> be subject to that.)

Ok. Thank you.. 
I misunderstood "svgView". 
I will rework it again.

[Shanmuga Pandi, 2016-07-22 04:24:35]

shanmuga.m@samsung.com changed reviewers:
+ foolip@chromium.org

[foolip, 2016-07-22 04:38:24]

foolip@chromium.org changed reviewers:
+ davve@opera.com, pdr@chromium.org

[foolip, 2016-07-22 04:44:39]

https://codereview.chromium.org/2163213007/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGSVGElement.cpp (left):

https://codereview.chromium.org/2163213007/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGSVGElement.cpp:661: if
(fragmentIdentifier.startsWith("svgView(")) {
On 2016/07/21 13:08:27, fs wrote:
> This is still in the spec AFAIK. the only thing that should be removed here
is:
> 
>  * the SVGViewSpec interface
>  * the SVGSVGElement.useCurrentView and SVGSVGElement.currentView
> 
> i.e the DOM exposed bits. Any cleanup that isn't obvious "JS only" (like
> removing the ScriptWrappable and related bits; *FromJavascript methods;
> obviously dead code) is best done as a follow-up (and the removal here would
not
> be subject to that.)

Hmm, so this gets at the same thing I'm wondering about on blink-dev. To remove
only the web-exposed bits of this but keeping all the internals, has any
meaningful simplification been achieved?

I looked around in cs.chromium.org but am not really familiar with any of the
history, so what would happen if SVGViewSpec and SVGViewElement were removed
entirely?

(I added davve@ and pdr@ in case they have thoughts on the matter.)

[fs, 2016-07-22 08:31:38]

https://codereview.chromium.org/2163213007/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGSVGElement.cpp (left):

https://codereview.chromium.org/2163213007/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGSVGElement.cpp:661: if
(fragmentIdentifier.startsWith("svgView(")) {
On 2016/07/22 at 04:44:39, foolip wrote:
> On 2016/07/21 13:08:27, fs wrote:
> > This is still in the spec AFAIK. the only thing that should be removed here
is:
> > 
> >  * the SVGViewSpec interface
> >  * the SVGSVGElement.useCurrentView and SVGSVGElement.currentView
> > 
> > i.e the DOM exposed bits. Any cleanup that isn't obvious "JS only" (like
> > removing the ScriptWrappable and related bits; *FromJavascript methods;
> > obviously dead code) is best done as a follow-up (and the removal here would
not
> > be subject to that.)
> 
> Hmm, so this gets at the same thing I'm wondering about on blink-dev. To
remove only the web-exposed bits of this but keeping all the internals, has any
meaningful simplification been achieved?
> 
> I looked around in cs.chromium.org but am not really familiar with any of the
history, so what would happen if SVGViewSpec and SVGViewElement were removed
entirely?

Just of the top of my head, some simplifications we could do:

 * Simpler types. Since we no longer need to expose the "exact" semantics of
SVGTransformList et.c, we could use a more efficient (and thus simpler) internal
representation.

 * Slightly simpler lifetimes. Oilpan fixed this quite a bit already (and there
are bits that are dead already from that - like
SVGViewSpec::detachContextElement.)

 * Removing SVGViewElement probably means we can simplify the setup of the (now
internal-only) SVGViewSpec (less need for things like
inheritViewAttributesFromElement, although removing the SVGViewSpec interface is
likely the bigger help there.)

 * With both gone we'd only have a single user of SVGZoomAndPan, and it's
possible we do something with that. (I'd be a bit surprised if anyone actually
used the functionality that is exposes though...)

When you start pulling the strings, it's quite possible more things fall off as
well.

[Shanmuga Pandi, 2016-07-26 07:06:09]

On 2016/07/22 08:31:38, fs (OoO until Aug 15) wrote:
>
https://codereview.chromium.org/2163213007/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/svg/SVGSVGElement.cpp (left):
> 
>
https://codereview.chromium.org/2163213007/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/svg/SVGSVGElement.cpp:661: if
> (fragmentIdentifier.startsWith("svgView(")) {
> On 2016/07/22 at 04:44:39, foolip wrote:
> > On 2016/07/21 13:08:27, fs wrote:
> > > This is still in the spec AFAIK. the only thing that should be removed
here
> is:
> > > 
> > >  * the SVGViewSpec interface
> > >  * the SVGSVGElement.useCurrentView and SVGSVGElement.currentView
> > > 
> > > i.e the DOM exposed bits. Any cleanup that isn't obvious "JS only" (like
> > > removing the ScriptWrappable and related bits; *FromJavascript methods;
> > > obviously dead code) is best done as a follow-up (and the removal here
would
> not
> > > be subject to that.)
> > 
> > Hmm, so this gets at the same thing I'm wondering about on blink-dev. To
> remove only the web-exposed bits of this but keeping all the internals, has
any
> meaningful simplification been achieved?
> > 
> > I looked around in http://cs.chromium.org but am not really familiar with
any of the
> history, so what would happen if SVGViewSpec and SVGViewElement were removed
> entirely?
> 
> Just of the top of my head, some simplifications we could do:
> 
>  * Simpler types. Since we no longer need to expose the "exact" semantics of
> SVGTransformList et.c, we could use a more efficient (and thus simpler)
internal
> representation.
> 
>  * Slightly simpler lifetimes. Oilpan fixed this quite a bit already (and
there
> are bits that are dead already from that - like
> SVGViewSpec::detachContextElement.)
> 
>  * Removing SVGViewElement probably means we can simplify the setup of the
(now
> internal-only) SVGViewSpec (less need for things like
> inheritViewAttributesFromElement, although removing the SVGViewSpec interface
is
> likely the bigger help there.)
> 
>  * With both gone we'd only have a single user of SVGZoomAndPan, and it's
> possible we do something with that. (I'd be a bit surprised if anyone actually
> used the functionality that is exposes though...)
> 
> When you start pulling the strings, it's quite possible more things fall off
as
> well.

As the first step, shall I remove the SVGViewSpec interface and the
SVGSVGElement.useCurrentView and SVGSVGElement.currentView as follwing the spec?
 And remove deadCode if any.
I will keep #svgView() and SVGViewElement now.

[Shanmuga Pandi, 2016-07-26 10:50:39]

PTAL!!

[Shanmuga Pandi, 2016-07-26 10:54:51]

The CQ bit was checked by shanmuga.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-26 10:55:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 12:08:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-26 12:08:15]

Dry run: This issue passed the CQ dry run.

[fs, 2016-07-26 16:09:57]

On 2016/07/26 at 07:06:09, shanmuga.m wrote:
> On 2016/07/22 08:31:38, fs (OoO until Aug 15) wrote:
> >
https://codereview.chromium.org/2163213007/diff/1/third_party/WebKit/Source/c...
> > File third_party/WebKit/Source/core/svg/SVGSVGElement.cpp (left):
> > 
> >
https://codereview.chromium.org/2163213007/diff/1/third_party/WebKit/Source/c...
> > third_party/WebKit/Source/core/svg/SVGSVGElement.cpp:661: if
> > (fragmentIdentifier.startsWith("svgView(")) {
> > On 2016/07/22 at 04:44:39, foolip wrote:
> > > On 2016/07/21 13:08:27, fs wrote:
> > > > This is still in the spec AFAIK. the only thing that should be removed
here
> > is:
> > > > 
> > > >  * the SVGViewSpec interface
> > > >  * the SVGSVGElement.useCurrentView and SVGSVGElement.currentView
> > > > 
> > > > i.e the DOM exposed bits. Any cleanup that isn't obvious "JS only" (like
> > > > removing the ScriptWrappable and related bits; *FromJavascript methods;
> > > > obviously dead code) is best done as a follow-up (and the removal here
would
> > not
> > > > be subject to that.)
> > > 
> > > Hmm, so this gets at the same thing I'm wondering about on blink-dev. To
> > remove only the web-exposed bits of this but keeping all the internals, has
any
> > meaningful simplification been achieved?
> > > 
> > > I looked around in http://cs.chromium.org but am not really familiar with
any of the
> > history, so what would happen if SVGViewSpec and SVGViewElement were removed
> > entirely?
> > 
> > Just of the top of my head, some simplifications we could do:
> > 
> >  * Simpler types. Since we no longer need to expose the "exact" semantics of
> > SVGTransformList et.c, we could use a more efficient (and thus simpler)
internal
> > representation.
> > 
> >  * Slightly simpler lifetimes. Oilpan fixed this quite a bit already (and
there
> > are bits that are dead already from that - like
> > SVGViewSpec::detachContextElement.)
> > 
> >  * Removing SVGViewElement probably means we can simplify the setup of the
(now
> > internal-only) SVGViewSpec (less need for things like
> > inheritViewAttributesFromElement, although removing the SVGViewSpec
interface is
> > likely the bigger help there.)
> > 
> >  * With both gone we'd only have a single user of SVGZoomAndPan, and it's
> > possible we do something with that. (I'd be a bit surprised if anyone
actually
> > used the functionality that is exposes though...)
> > 
> > When you start pulling the strings, it's quite possible more things fall off
as
> > well.
> 
> As the first step, shall I remove the SVGViewSpec interface and the
SVGSVGElement.useCurrentView and SVGSVGElement.currentView as follwing the spec?
 And remove deadCode if any.
> I will keep #svgView() and SVGViewElement now.

SGTM. This CL looks good to me barring that the intent process completes.

[fs, 2016-07-26 16:10:10]

https://codereview.chromium.org/2163213007/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGViewSpec-defaults.js
(left):

https://codereview.chromium.org/2163213007/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGViewSpec-defaults.js:1:
description("This test checks the SVGViewSpec API");
This test probably has a corresponding -expected.txt file that should be removed
too?

https://codereview.chromium.org/2163213007/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGViewSpec.h (right):

https://codereview.chromium.org/2163213007/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGViewSpec.h:42: // JS API
Nit: Drop this comment

[Shanmuga Pandi, 2016-07-28 05:22:17]

On 2016/07/26 16:10:10, fs (OoO until Aug 15) wrote:
>
https://codereview.chromium.org/2163213007/diff/40001/third_party/WebKit/Layo...
> File
third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGViewSpec-defaults.js
> (left):
> 
>
https://codereview.chromium.org/2163213007/diff/40001/third_party/WebKit/Layo...
> third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGViewSpec-defaults.js:1:
> description("This test checks the SVGViewSpec API");
> This test probably has a corresponding -expected.txt file that should be
removed
> too?
> 
>
https://codereview.chromium.org/2163213007/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/svg/SVGViewSpec.h (right):
> 
>
https://codereview.chromium.org/2163213007/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/svg/SVGViewSpec.h:42: // JS API
> Nit: Drop this comment

Done!!

[foolip, 2016-07-29 02:07:59]

Can you also remove the entries inUseCounter.h that become unused due to the IDL
file changes? (I'm still trying to figure out what to do in the intent thread,
thanks for your patience...)

[Shanmuga Pandi, 2016-07-29 12:33:47]

On 2016/07/29 02:07:59, foolip wrote:
> Can you also remove the entries inUseCounter.h that become unused due to the
IDL
> file changes? (I'm still trying to figure out what to do in the intent thread,
> thanks for your patience...)

Removed the entries!!
Thank you for your comments!

[Shanmuga Pandi, 2016-08-01 11:11:44]

Description was changed from

==========
Remove currentView, useCurrentView properties of SVGSVGElement and SVGViewSpec
interface

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Dfqld7wjHJM

BUG=629445
==========

to

==========
Deprecate currentView, useCurrentView properties of SVGSVGElement and
SVGViewSpec interface

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Dfqld7wjHJM

BUG=629445
==========

[Shanmuga Pandi, 2016-08-01 11:14:18]

PTAL!!

I have deprecated instead Remove!!

Thanks

[Shanmuga Pandi, 2016-08-01 11:15:03]

The CQ bit was checked by shanmuga.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-01 11:15:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[foolip, 2016-08-01 11:41:09]

lgtm

[commit-bot: I haz the power, 2016-08-01 12:43:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-01 12:43:53]

Dry run: This issue passed the CQ dry run.

[Shanmuga Pandi, 2016-08-01 13:22:06]

On 2016/08/01 11:41:09, foolip wrote:
> lgtm

Thank you.. 
Shall I proceed to commit, Since We got enough LGTMs on Blink-dev thread ?

[foolip, 2016-08-01 13:26:16]

On 2016/08/01 13:22:06, Shanmuga Pandi wrote:
> On 2016/08/01 11:41:09, foolip wrote:
> > lgtm
> 
> Thank you.. 
> Shall I proceed to commit, Since We got enough LGTMs on Blink-dev thread ?

Yes, please go ahead.

[Shanmuga Pandi, 2016-08-01 14:15:03]

The CQ bit was checked by shanmuga.m@samsung.com

[commit-bot: I haz the power, 2016-08-01 14:15:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-01 14:19:08]

Description was changed from

==========
Deprecate currentView, useCurrentView properties of SVGSVGElement and
SVGViewSpec interface

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Dfqld7wjHJM

BUG=629445
==========

to

==========
Deprecate currentView, useCurrentView properties of SVGSVGElement and
SVGViewSpec interface

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Dfqld7wjHJM

BUG=629445
==========

[commit-bot: I haz the power, 2016-08-01 14:19:10]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-08-01 14:22:11]

Description was changed from

==========
Deprecate currentView, useCurrentView properties of SVGSVGElement and
SVGViewSpec interface

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Dfqld7wjHJM

BUG=629445
==========

to

==========
Deprecate currentView, useCurrentView properties of SVGSVGElement and
SVGViewSpec interface

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Dfqld7wjHJM

BUG=629445

Committed: https://crrev.com/11cc1db033d902c13c429f8bee0a13b6381c8e4e
Cr-Commit-Position: refs/heads/master@{#408958}
==========

[commit-bot: I haz the power, 2016-08-01 14:22:13]

Patchset 6 (id:??) landed as
https://crrev.com/11cc1db033d902c13c429f8bee0a13b6381c8e4e
Cr-Commit-Position: refs/heads/master@{#408958}