Closed Bug 869961 Opened 7 years ago Closed 7 years ago

RILContentHelper javascript error when running xul-runner script


(Firefox OS Graveyard :: General, defect)

Not set


(Not tracked)



(Reporter: jhylands, Assigned: jhylands)


(Keywords: perf, Whiteboard: c= p=3 [fixed-in-birch])


(4 files, 2 obsolete files)

When we generate reference workloads, we do so using a RIL-enabled xulrunner script in a b2g-desktop repository. When phone numbers are parsed as a part of generating the contacts database, the getter function voiceConnectionInfo() in RILContentHelper.js is called. This function is a one-liner:

  return this.getRilContext().voiceConnectionInfo;

In this setup, this.getRilContext() returns null. It is reasonable (and is checked for in the caller) for this function to return null.
Attachment #747031 - Flags: review?(htsai)
Whiteboard: c=performance
Assignee: nobody → jhylands
Whiteboard: c=performance → c=performance p=3
Whiteboard: c=performance p=3 → c= p=3
Comment on attachment 747031 [details] [diff] [review]
Patch for checking for null context

Review of attachment 747031 [details] [diff] [review]:

Thanks for the patch, Jon. The patch is on the right way, but we need to do more. Please see my comments below.

::: dom/system/gonk/RILContentHelper.js
@@ +417,5 @@
> +    var context = this.getRilContext();
> +    if (context)
> +      return context.voiceConnectionInfo;
> +    else
> +      return null;

Let's do in this way:

let context = this.getRilContext();
return context ? context.voiceConnectionInfo : null;

We also need to apply modification to
|get iccInfo|, |get dataConnectionInfo|, |get cardState|, |get retryCount| and | get networkSelectionMode|.
Attachment #747031 - Flags: review?(htsai)
Hi, Jon
Can you show me how to run xulrunner ?
I'd like to know why rilContext is null in this case.

Attachment #747031 - Attachment is obsolete: true
Attachment #747405 - Flags: review?(htsai)

Its kind of complicated to get set up, but I'll see what I can do to explain it. This only seems to happen on m-c though - it doesn't happen on either b2g18 or 1.0.1 (the other two places I've run this). I'll attach the files needed in order to run the script. You can do this all in a b2g-desktop repo on your PC.
Insert the contents of this file somewhere reasonable inside your b2g-desktop gaia Makefile. Your gaia repo should be synced off master.
Save this file to b2g-desktop/gaia/build
Under b2g-desktop/gaia/test_media, create a directory named 'reference-workload', and save this file into that directory.
Once you have those files setup/saved, make sure you have run |make -f| from b2g-desktop/mozilla-central to create the b2g-desktop/build directory, and then, from a command line in the gaia directory, run:

make fake-contacts

You should run into the error immediately.
Note that in comment 6, you have to adjust one line to point to your b2g-desktop build folder:

Comment on attachment 747405 [details] [diff] [review]
Patch for checking for null context

Review of attachment 747405 [details] [diff] [review]:

r=me with the comment addressed.

Thanks again for the catch and the patch, Jon :)

::: dom/system/gonk/RILContentHelper.js
@@ +410,5 @@
>    },
>    get iccInfo() {
> +    let context = this.getRilContext();
> +    return context ? context.iccInfo : null;

Sorry for change again here. :(

To fit the coding style we are using in RIL more on second thought, please help change to |return context && context.iccInfo;|, here and the following! Thanks!
Attachment #747405 - Flags: review?(htsai) → review+
Hsinyi - I've updated the patch. Note that I don't have permissions to merge, so can you or someone on your team take care of it please?
Attachment #747405 - Attachment is obsolete: true
(In reply to Jon Hylands [:jhylands] from comment #12)
> Created attachment 747934 [details] [diff] [review]
> Patch for checking for null context
> Hsinyi - I've updated the patch. Note that I don't have permissions to
> merge, so can you or someone on your team take care of it please?
Sure, happy to do that!
Whiteboard: c= p=3 → c= p=3, [fixed-in-birch]
Closed: 7 years ago
Resolution: --- → FIXED
Fixing p=X whiteboard so points are retained in
Keywords: perf
Whiteboard: c= p=3, [fixed-in-birch] → c= p=3 [fixed-in-birch]
You need to log in before you can comment on or make changes to this bug.