[DevTools] Emulate hover and pointer types.

[DevTools] Emulate hover and pointer types.

When emulating mobile device, we use PointerTypeCoarse and HoverTypeOnDemand.

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

[dgozman, 2015-09-15 01:16:39]

dgozman@chromium.org changed reviewers:
+ mustaq@chromium.org, pfeldman@chromium.org, rbyers@chromium.org

[dgozman, 2015-09-15 01:16:40]

Hi, could you please take a look?

With this patch @media (pointer: coarse) and @media (hover: on-demand) apply
without reload. That should be an effect of the mediaQueryAffectingValueChanged
call we already do for emulation:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Thanks,
Dmitry

[mustaq, 2015-09-15 14:29:03]

https://codereview.chromium.org/1343843004/diff/20001/Source/web/DevToolsEmul...
File Source/web/DevToolsEmulator.cpp (right):

https://codereview.chromium.org/1343843004/diff/20001/Source/web/DevToolsEmul...
Source/web/DevToolsEmulator.cpp:65: ,
m_embedderAvailablePointerTypes(webViewImpl->page()->settings().availableHoverTypes())
s/Hover/Pointer/

https://codereview.chromium.org/1343843004/diff/20001/Source/web/DevToolsEmul...
Source/web/DevToolsEmulator.cpp:204: return m_embedderAvailablePointerTypes;
I think the getters should behave similarly to the setters here, right? I mean
they should return emulated values when mobile emulation is enabled?

[dgozman, 2015-09-15 18:18:05]

https://codereview.chromium.org/1343843004/diff/20001/Source/web/DevToolsEmul...
File Source/web/DevToolsEmulator.cpp (right):

https://codereview.chromium.org/1343843004/diff/20001/Source/web/DevToolsEmul...
Source/web/DevToolsEmulator.cpp:65: ,
m_embedderAvailablePointerTypes(webViewImpl->page()->settings().availableHoverTypes())
On 2015/09/15 14:29:03, mustaq wrote:
> s/Hover/Pointer/

Ops. Done.

https://codereview.chromium.org/1343843004/diff/20001/Source/web/DevToolsEmul...
Source/web/DevToolsEmulator.cpp:204: return m_embedderAvailablePointerTypes;
On 2015/09/15 14:29:03, mustaq wrote:
> I think the getters should behave similarly to the setters here, right? I mean
> they should return emulated values when mobile emulation is enabled?

That's a tricky question. When mobile emulation is enabled, any call to setXXX
will be essentially ignored (since emulated value is used). That means getter
will return value different from what was set, which is confusing for the
client.

I'm actually surprised we have getters for settings. That's not a common
pattern, and I've found the only use among these 4 in the whole codebase. Should
we remove them instead?

[pfeldman, 2015-09-15 21:44:18]

lgtm

[mustaq, 2015-09-16 13:47:08]

lgtm

https://codereview.chromium.org/1343843004/diff/20001/Source/web/DevToolsEmul...
File Source/web/DevToolsEmulator.cpp (right):

https://codereview.chromium.org/1343843004/diff/20001/Source/web/DevToolsEmul...
Source/web/DevToolsEmulator.cpp:204: return m_embedderAvailablePointerTypes;
On 2015/09/15 18:18:05, dgozman wrote:
> On 2015/09/15 14:29:03, mustaq wrote:
> > I think the getters should behave similarly to the setters here, right? I
mean
> > they should return emulated values when mobile emulation is enabled?
> 
> That's a tricky question. When mobile emulation is enabled, any call to setXXX
> will be essentially ignored (since emulated value is used). That means getter
> will return value different from what was set, which is confusing for the
> client.
> 
> I'm actually surprised we have getters for settings. That's not a common
> pattern, and I've found the only use among these 4 in the whole codebase.
Should
> we remove them instead?

Yes, I think removing them makes sense since they are unused. Thanks.

[Rick Byers, 2015-09-16 14:03:29]

LGTM also, thanks

[dgozman, 2015-09-17 18:54:31]

The CQ bit was checked by dgozman@chromium.org

[dgozman, 2015-09-17 18:54:32]

The patchset sent to the CQ was uploaded after l-g-t-m from mustaq@chromium.org,
pfeldman@chromium.org, rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/1343843004/#ps60001
(title: "removed settings getters")

[commit-bot: I haz the power, 2015-09-17 18:54:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1343843004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1343843004/60001

[commit-bot: I haz the power, 2015-09-17 20:23:54]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=202468