Update WebTelephony impl to use the new DOMEventTargetHelper API

RESOLVED FIXED in mozilla14

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: smaug, Assigned: fabrice)

Tracking

unspecified
mozilla14
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.12 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
smaug
: review+
Details | Diff | Splinter Review
Comment hidden (empty)
(Reporter)

Updated

5 years ago
Summary: Update B2G EventTarget objects to use changed DOMEventTargetHelper API → Update B2G EventTarget objects to use the new DOMEventTargetHelper API
smaug, which ones are you specifically talking about?
(Reporter)

Comment 2

5 years ago
Ask fabrice. I'm not familiar with b2g code, but apparently there are some
classes which extend nsDOMEventTargetHelper.
(Assignee)

Comment 3

5 years ago
Created attachment 605466 [details] [diff] [review]
patch

This patch fixes the build issues, but I'm not sure *at all* it's doing the right thing.
Attachment #605466 - Flags: feedback?(bugs)
(Reporter)

Comment 4

5 years ago
Comment on attachment 605466 [details] [diff] [review]
patch

>+++ b/dom/telephony/Telephony.h
>@@ -118,16 +118,12 @@ public:
>     return mRIL;
>   }
> 
>-  nsPIDOMWindow*
>-  Owner() const
>+  nsIScriptContext*
>+  ScriptContext()
>   {
>-    return mOwner;
>-  }
>-
>-  nsIScriptContext*
>-  ScriptContext() const
>-  {
>-    return mScriptContext;
>+    nsresult rv;
>+    nsIScriptContext* sc = GetContextForEventHandlers(&rv);
>+    return sc;
>   }
I don't know why you some times use GetContextForEventHandlers, and yet have
this helper method.


> 
>   nsRefPtr<TelephonyCall> call = new TelephonyCall();
> 
>-  call->mOwner = aTelephony->Owner();
>-  call->mScriptContext = aTelephony->ScriptContext();
>   call->mTelephony = aTelephony;
>   call->mNumber = aNumber;
You don't bind the object to owner?
Attachment #605466 - Flags: feedback?(bugs) → feedback+
(Assignee)

Comment 5

5 years ago
Created attachment 605475 [details] [diff] [review]
patch v2

updated patch, thanks Olli for the quick review on the first one!
Assignee: nobody → fabrice
Attachment #605466 - Attachment is obsolete: true
Attachment #605475 - Flags: review?(bugs)
Attachment #605475 - Flags: review?(bent.mozilla)
(Reporter)

Updated

5 years ago
Attachment #605475 - Flags: review?(bugs) → review+
Blocks: 674726
Component: General → DOM: Device Interfaces
Product: Boot2Gecko → Core
QA Contact: general → device-interfaces
Summary: Update B2G EventTarget objects to use the new DOMEventTargetHelper API → Update WebTelephony impl to use the new DOMEventTargetHelper API
(Assignee)

Updated

5 years ago
Blocks: 719491
Comment on attachment 605475 [details] [diff] [review]
patch v2

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

r=me with this addressed:

::: dom/telephony/Telephony.cpp
@@ +329,5 @@
>    }
>  
> +  nsresult rv;
> +  nsIScriptContext* sc = GetContextForEventHandlers(&rv);
> +  if (NS_SUCCEEDED(rv) && sc) {

Why not do NS_ENSURE_SUCCESS(rv, rv) here?

@@ +346,5 @@
>    JSObject* calls = mCallsArray;
>    if (!calls) {
> +    nsresult rv;
> +    nsIScriptContext* sc = GetContextForEventHandlers(&rv);
> +    if (NS_SUCCEEDED(rv) && sc) {

Here too.
Attachment #605475 - Flags: review?(bent.mozilla) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/c71845b3b2a6
(Assignee)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.