864206 - Use dom::Touch instead of nsIDOMTouch where possible

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[David Zbarsky (:dzbarsky)]

Attachment: Patch

Comment 1

[David Zbarsky (:dzbarsky)]

Attachment: Move variables from nsIDOMTouch to Touch

Comment 2

[Olli Pettay [:smaug]]

You must update uuid of nsIDOMTouch

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 740139 [details] [diff] [review]
Patch

Review of attachment 740139 [details] [diff] [review]:
-----------------------------------------------------------------

What made the extra includes necessary?

::: content/events/src/nsDOMTouchEvent.cpp
@@ +16,5 @@
>  using namespace mozilla;
>  using namespace mozilla::dom;
>  
>  // TouchList
> +nsDOMTouchList::nsDOMTouchList(nsTArray<nsRefPtr<Touch> > &aTouches)

& to the left

@@ +56,5 @@
>    *aRetVal = nullptr;
>    for (uint32_t i = 0; i < mPoints.Length(); ++i) {
> +    nsRefPtr<Touch> point = mPoints[i];
> +    if (point && aIdentifier == point->Identifier()) {
> +      *aRetVal = point.forget().get();

point.forget(aRetVal);

::: js/xpconnect/src/dom_quickstubs.qsconf
@@ -149,5 @@
>      'nsIDOMBlob': 'nsIDOMFile',
>  
>      'nsIIndexedDatabaseUsageCallback': 'nsIIndexedDatabaseManager',
>  
> -    'nsIDOMTouch': 'nsIDOMTouchEvent',

Last time I tried this, the nsIDOMTouchList quickstubs were unhappy with it.

::: layout/base/nsPresShell.cpp
@@ +5899,5 @@
>  
>  static PLDHashOperator
> +AppendToTouchList(const uint32_t& aKey, nsRefPtr<Touch>& aData, void *aTouchList)
> +{
> +  nsTArray<nsRefPtr<Touch> > *touches = static_cast<nsTArray<nsRefPtr<Touch> > *>(aTouchList);

* to the left

@@ +6285,2 @@
>          for (uint32_t i = 0; i < touches.Length(); ++i) {
> +          Touch *touch = touches[i];

Ditto

@@ +6298,2 @@
>            nsCOMPtr<nsIDOMEventTarget> targetPtr;
>            oldTouch->GetTarget(getter_AddRefs(targetPtr));

Fix this too?

@@ +6673,5 @@
>        case NS_TOUCH_END: {
>          // Remove the changed touches
>          // need to make sure we only remove touches that are ending here
>          nsTouchEvent* touchEvent = static_cast<nsTouchEvent*>(aEvent);
> +        nsTArray<nsRefPtr<Touch> >  &touches = touchEvent->touches;

& to the left, and fix the double space

@@ +6678,2 @@
>          for (uint32_t i = 0; i < touches.Length(); ++i) {
> +          Touch *touch = touches[i];

* to the left

@@ +6706,4 @@
>          bool haveChanged = false;
>          for (int32_t i = touches.Length(); i; ) {
>            --i;
> +          Touch *touch = touches[i];

Ditto

@@ +6718,5 @@
>            if (!oldTouch) {
>              touches.RemoveElementAt(i);
>              continue;
>            }
> +          if(touch->Equals(oldTouch)) {

'if ('

@@ +6870,5 @@
>    nsTouchEvent* touchEvent = static_cast<nsTouchEvent*>(aEvent);
>  
>    // loop over all touches and dispatch events on any that have changed
>    for (uint32_t i = 0; i < touchEvent->touches.Length(); ++i) {
> +    Touch *touch = touchEvent->touches[i];

*

::: widget/windows/winrt/MetroInput.h
@@ +27,5 @@
> +namespace mozilla {
> +  namespace dom {
> +    class Touch;
> +  }
> +}

No need for the indentation, even if the following code is wrong

@@ +235,5 @@
>    nsTouchEvent mTouchEvent;
>    void DispatchPendingTouchEvent();
>    void DispatchPendingTouchEvent(nsEventStatus& status);
>    nsBaseHashtable<nsUint32HashKey,
> +                  nsCOMPtr<mozilla::dom::Touch>,

nsCOMPtr?

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 740149 [details] [diff] [review]
Move variables from nsIDOMTouch to Touch

Review of attachment 740149 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/events/src/Touch.h
@@ +116,3 @@
>    bool mPointsInitialized;
> +public:
> +  bool mChanged;

Please tell me people don't rely on these being public...

Comment 5

[David Zbarsky (:dzbarsky)]

(In reply to :Ms2ger from comment #3)
> Comment on attachment 740139 [details] [diff] [review]
> Patch
> 
> Review of attachment 740139 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> What made the extra includes necessary?
> 

Not including nsDOMUIEvent.h in Touch.h

(In reply to :Ms2ger from comment #4)
> Comment on attachment 740149 [details] [diff] [review]
> Move variables from nsIDOMTouch to Touch
> 
> Review of attachment 740149 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/events/src/Touch.h
> @@ +116,3 @@
> >    bool mPointsInitialized;
> > +public:
> > +  bool mChanged;
> 
> Please tell me people don't rely on these being public...

Yup...

Comment 6

[David Zbarsky (:dzbarsky)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43872f1bfbda

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/43872f1bfbda

Comment 8

[David Zbarsky (:dzbarsky)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43c0c4dd5a5c

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/43c0c4dd5a5c