Remove DelegatedFrameData::device_scale_factor

Remove DelegatedFrameData::device_scale_factor

Device scale factor appeared TWICE inside a CompositorFrame:
  1. CompositorFrameMetadata
  2. DelegatedFrameData

The version in DelegatedFrameData seemed largely dead except for some
Mus code so I've updated Mus code.

BUG=611802
TBR=dpranke@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/e9fab4366d0270c7879f6e40ef73233626c9904d
Cr-Commit-Position: refs/heads/master@{#402590}

[Fady Samuel, 2016-06-27 15:51:58]

Description was changed from

==========
Remove DelegatedFrameData::device_scale_factor

Device scale factor appeared TWICE inside a CompositorFrame:
  1. CompositorFrameMetadata
  2. DelegatedFrameData

The version in DelegatedFrameData seemed largely dead except for some
Mus code so I've updated Mus code.

BUG=611802
==========

to

==========
Remove DelegatedFrameData::device_scale_factor

Device scale factor appeared TWICE inside a CompositorFrame:
  1. CompositorFrameMetadata
  2. DelegatedFrameData

The version in DelegatedFrameData seemed largely dead except for some
Mus code so I've updated Mus code.

BUG=611802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-06-27 15:52:21]

The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-27 15:52:47]

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

[Fady Samuel, 2016-06-27 15:53:04]

fsamuel@chromium.org changed reviewers:
+ dcheng@chromium.org, piman@chromium.org, sky@chromium.org

[Fady Samuel, 2016-06-27 15:53:05]

+piman@ for cc
+dcheng@ for security (I may TBR you since this is a pure code removal)
+sky@ for mus

[piman, 2016-06-27 15:58:33]

If it's not used any more, then LGTM.

[commit-bot: I haz the power, 2016-06-27 16:10:50]

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

[commit-bot: I haz the power, 2016-06-27 16:10:51]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[sky, 2016-06-27 16:17:21]

LGTM

[Fady Samuel, 2016-06-27 16:44:41]

fsamuel@chromium.org changed reviewers:
+ boliu@chromium.org

[Fady Samuel, 2016-06-27 16:44:42]

+boliu@ for android_webivew

[Fady Samuel, 2016-06-27 16:44:53]

The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-27 16:45:29]

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-06-27 16:48:15]

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

[commit-bot: I haz the power, 2016-06-27 16:48:16]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Fady Samuel, 2016-06-27 17:00:26]

The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-27 17:02:05]

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

[boliu, 2016-06-27 17:05:28]

boliu@chromium.org changed reviewers:
+ danakj@chromium.org

[boliu, 2016-06-27 17:05:28]

piman/danakj: Did desktop browser compositor switch from DIP to physical pixels
in the last 2 years, ie since https://codereview.chromium.org/336433003 landed?

The lines in android that sets the scale to 1 definitely were not no-op back
when it was first added. Then found this comment from dana about the desktop
browser compositor:
https://bugs.chromium.org/p/chromium/issues/detail?id=384134#c4 perhaps it
already happened?

I assume so, in which case the correct change is to just remove these lines. In
particular, the change in render_widget_host_view_android.cc right now is
definitely wrong because there are things that reads the page scale in the
metadata.

[dcheng, 2016-06-27 17:07:07]

IPC lgtm

[danakj, 2016-06-27 17:15:59]

On Mon, Jun 27, 2016 at 10:05 AM, <boliu@chromium.org> wrote:

> piman/danakj: Did desktop browser compositor switch from DIP to physical
> pixels
> in the last 2 years, ie since https://codereview.chromium.org/336433003
> landed?
>

Nope. Still dip. Blink's compositor in DSF is still behind a flag also.


> The lines in android that sets the scale to 1 definitely were not no-op
> back
> when it was first added. Then found this comment from dana about the
> desktop
> browser compositor:
> https://bugs.chromium.org/p/chromium/issues/detail?id=384134#c4 perhaps it
> already happened?
>
> I assume so, in which case the correct change is to just remove these
> lines. In
> particular, the change in render_widget_host_view_android.cc right now is
> definitely wrong because there are things that reads the page scale in the
> metadata.
>
> https://codereview.chromium.org/2101623002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[piman, 2016-06-27 17:30:27]

On 2016/06/27 17:15:59, danakj wrote:
> On Mon, Jun 27, 2016 at 10:05 AM, <mailto:boliu@chromium.org> wrote:
> 
> > piman/danakj: Did desktop browser compositor switch from DIP to physical
> > pixels
> > in the last 2 years, ie since https://codereview.chromium.org/336433003
> > landed?
> >
> 
> Nope. Still dip. Blink's compositor in DSF is still behind a flag also.
> 
> 
> > The lines in android that sets the scale to 1 definitely were not no-op
> > back
> > when it was first added. Then found this comment from dana about the
> > desktop
> > browser compositor:
> > https://bugs.chromium.org/p/chromium/issues/detail?id=384134#c4 perhaps it
> > already happened?
> >
> > I assume so, in which case the correct change is to just remove these
> > lines. In
> > particular, the change in render_widget_host_view_android.cc right now is
> > definitely wrong because there are things that reads the page scale in the
> > metadata.
> >
> > https://codereview.chromium.org/2101623002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Right, I don't think we should be changing the metadata, which is still used to
match frames during DSF transitions (e.g. moving window between screens) AFAIU,
even with use-zoom-for-dsf

[commit-bot: I haz the power, 2016-06-27 18:04:33]

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

[commit-bot: I haz the power, 2016-06-27 18:04:34]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[boliu, 2016-06-27 18:35:24]

On 2016/06/27 17:15:59, danakj wrote:
> On Mon, Jun 27, 2016 at 10:05 AM, <mailto:boliu@chromium.org> wrote:
> 
> > piman/danakj: Did desktop browser compositor switch from DIP to physical
> > pixels
> > in the last 2 years, ie since https://codereview.chromium.org/336433003
> > landed?
> >
> 
> Nope. Still dip. Blink's compositor in DSF is still behind a flag also.

Semi OT, but what cc change made resetting the delegated frame scale to 1
unnecessary on android? Know off the top of your head?

> 
> 
> > The lines in android that sets the scale to 1 definitely were not no-op
> > back
> > when it was first added. Then found this comment from dana about the
> > desktop
> > browser compositor:
> > https://bugs.chromium.org/p/chromium/issues/detail?id=384134#c4 perhaps it
> > already happened?
> >
> > I assume so, in which case the correct change is to just remove these
> > lines. In
> > particular, the change in render_widget_host_view_android.cc right now is
> > definitely wrong because there are things that reads the page scale in the
> > metadata.
> >
> > https://codereview.chromium.org/2101623002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[danakj, 2016-06-27 18:40:11]

On Mon, Jun 27, 2016 at 11:35 AM, <boliu@chromium.org> wrote:

> On 2016/06/27 17:15:59, danakj wrote:
> > On Mon, Jun 27, 2016 at 10:05 AM, <mailto:boliu@chromium.org> wrote:
> >
> > > piman/danakj: Did desktop browser compositor switch from DIP to
> physical
> > > pixels
> > > in the last 2 years, ie since
> https://codereview.chromium.org/336433003
> > > landed?
> > >
> >
> > Nope. Still dip. Blink's compositor in DSF is still behind a flag also.
>
> Semi OT, but what cc change made resetting the delegated frame scale to 1
> unnecessary on android? Know off the top of your head?
>

Well, DelegatedRendererLayerImpl doesn't exist anymore. Maybe probably
switching to surfaces changed this requirement.


>
>
> >
> >
> > > The lines in android that sets the scale to 1 definitely were not no-op
> > > back
> > > when it was first added. Then found this comment from dana about the
> > > desktop
> > > browser compositor:
> > > https://bugs.chromium.org/p/chromium/issues/detail?id=384134#c4
> perhaps it
> > > already happened?
> > >
> > > I assume so, in which case the correct change is to just remove these
> > > lines. In
> > > particular, the change in render_widget_host_view_android.cc right now
> is
> > > definitely wrong because there are things that reads the page scale in
> the
> > > metadata.
> > >
> > > https://codereview.chromium.org/2101623002/
> > >
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Chromium-reviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>
>
>
> https://codereview.chromium.org/2101623002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[boliu, 2016-06-27 18:52:49]

On 2016/06/27 18:40:11, danakj wrote:
> On Mon, Jun 27, 2016 at 11:35 AM, <mailto:boliu@chromium.org> wrote:
> 
> > On 2016/06/27 17:15:59, danakj wrote:
> > > On Mon, Jun 27, 2016 at 10:05 AM, <mailto:boliu@chromium.org> wrote:
> > >
> > > > piman/danakj: Did desktop browser compositor switch from DIP to
> > physical
> > > > pixels
> > > > in the last 2 years, ie since
> > https://codereview.chromium.org/336433003
> > > > landed?
> > > >
> > >
> > > Nope. Still dip. Blink's compositor in DSF is still behind a flag also.
> >
> > Semi OT, but what cc change made resetting the delegated frame scale to 1
> > unnecessary on android? Know off the top of your head?
> >
> 
> Well, DelegatedRendererLayerImpl doesn't exist anymore. Maybe probably
> switching to surfaces changed this requirement.
> 

Looks like it.

Ok, should just remove the reset code from android then.

> 
> >
> >
> > >
> > >
> > > > The lines in android that sets the scale to 1 definitely were not no-op
> > > > back
> > > > when it was first added. Then found this comment from dana about the
> > > > desktop
> > > > browser compositor:
> > > > https://bugs.chromium.org/p/chromium/issues/detail?id=384134#c4
> > perhaps it
> > > > already happened?
> > > >
> > > > I assume so, in which case the correct change is to just remove these
> > > > lines. In
> > > > particular, the change in render_widget_host_view_android.cc right now
> > is
> > > > definitely wrong because there are things that reads the page scale in
> > the
> > > > metadata.
> > > >
> > > > https://codereview.chromium.org/2101623002/
> > > >
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups
> > > "Chromium-reviews" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an
> > email
> > > to mailto:chromium-reviews+unsubscribe@chromium.org.
> >
> >
> >
> > https://codereview.chromium.org/2101623002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[Fady Samuel, 2016-06-28 16:05:56]

PTAL Bo, Dana! I just removed the resetting of the device scale factor!

[Fady Samuel, 2016-06-28 16:06:05]

The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run

[boliu, 2016-06-28 16:06:42]

lgtm

[commit-bot: I haz the power, 2016-06-28 16:06:47]

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-06-28 16:30:19]

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

[commit-bot: I haz the power, 2016-06-28 16:30:21]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[piman, 2016-06-28 17:25:17]

lgtm

[danakj, 2016-06-28 18:54:22]

LGTM

[Fady Samuel, 2016-06-28 19:48:55]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-06-28 19:48:56]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
dcheng@chromium.org, danakj@chromium.org, piman@chromium.org, boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2101623002/#ps80001
(title: "Fix fuzzer")

[commit-bot: I haz the power, 2016-06-28 19:49:42]

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-06-28 19:56:28]

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

[commit-bot: I haz the power, 2016-06-28 19:56:30]

Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/8...)

[Fady Samuel, 2016-06-28 20:12:26]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2016-06-28 20:13:59]

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-06-28 20:27:05]

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

[commit-bot: I haz the power, 2016-06-28 20:27:06]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Fady Samuel, 2016-06-28 20:55:59]

fsamuel@chromium.org changed reviewers:
+ dpranke@chromium.org

[Fady Samuel, 2016-06-28 20:56:00]

+dpranke@ for tools (I'm TBR'ing since it's just a removal).

[Fady Samuel, 2016-06-28 20:56:23]

Description was changed from

==========
Remove DelegatedFrameData::device_scale_factor

Device scale factor appeared TWICE inside a CompositorFrame:
  1. CompositorFrameMetadata
  2. DelegatedFrameData

The version in DelegatedFrameData seemed largely dead except for some
Mus code so I've updated Mus code.

BUG=611802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Remove DelegatedFrameData::device_scale_factor

Device scale factor appeared TWICE inside a CompositorFrame:
  1. CompositorFrameMetadata
  2. DelegatedFrameData

The version in DelegatedFrameData seemed largely dead except for some
Mus code so I've updated Mus code.

BUG=611802
TBR=dpranke@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Dirk Pranke, 2016-06-28 20:57:02]

lgtm

[Fady Samuel, 2016-06-28 20:57:31]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2016-06-28 20:59:00]

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-06-28 23:17:25]

Description was changed from

==========
Remove DelegatedFrameData::device_scale_factor

Device scale factor appeared TWICE inside a CompositorFrame:
  1. CompositorFrameMetadata
  2. DelegatedFrameData

The version in DelegatedFrameData seemed largely dead except for some
Mus code so I've updated Mus code.

BUG=611802
TBR=dpranke@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Remove DelegatedFrameData::device_scale_factor

Device scale factor appeared TWICE inside a CompositorFrame:
  1. CompositorFrameMetadata
  2. DelegatedFrameData

The version in DelegatedFrameData seemed largely dead except for some
Mus code so I've updated Mus code.

BUG=611802
TBR=dpranke@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-06-28 23:17:27]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-28 23:20:28]

Description was changed from

==========
Remove DelegatedFrameData::device_scale_factor

Device scale factor appeared TWICE inside a CompositorFrame:
  1. CompositorFrameMetadata
  2. DelegatedFrameData

The version in DelegatedFrameData seemed largely dead except for some
Mus code so I've updated Mus code.

BUG=611802
TBR=dpranke@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Remove DelegatedFrameData::device_scale_factor

Device scale factor appeared TWICE inside a CompositorFrame:
  1. CompositorFrameMetadata
  2. DelegatedFrameData

The version in DelegatedFrameData seemed largely dead except for some
Mus code so I've updated Mus code.

BUG=611802
TBR=dpranke@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/e9fab4366d0270c7879f6e40ef73233626c9904d
Cr-Commit-Position: refs/heads/master@{#402590}
==========

[commit-bot: I haz the power, 2016-06-28 23:20:29]

Patchset 5 (id:??) landed as
https://crrev.com/e9fab4366d0270c7879f6e40ef73233626c9904d
Cr-Commit-Position: refs/heads/master@{#402590}