1196652 - OriginSuffix is shown in about:serviceworker on b2g

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: sw.png

See the attached image,
The origin of the sw ends with ^appId=1048.
It should use originNoSuffix.

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

The principal is actually a plain JS object wrapped in serializeServiceWorkerInfo 
https://dxr.mozilla.org/mozilla-central/source/b2g/components/AboutServiceWorkers.jsm#37

Hi, Bobby

I can simply change the property 'origin' to 'originNoSuffix'.

But I saw there's already some code for Principal in NS_DOMWrite/ReadStructuredClone,
to make the principal in serviceworkerinfo is actually an nsIPrincipal I guess we also need to implement it in StackScopedCloneRead/Write, right?

Do you think we should implement structured clone for nsIPrincpal in StackScopedCloneRead/Write?

Thanks

Comment 2

[Bobby Holley (:bholley)]

Hi Yoshi,

Yes, cloning principals in StackScopedClone sounds like a great idea. Andrea recently unified a bunch of the structured cloning code. Andrea, can you help Yoshi figure out how to take advantage of the principal serialization code in StructuredCloneHelper.cpp from StackScopedClone?

Thanks!

Comment 3

[Andrea Marchesini [:baku]]

Hi Yoshi,

NS_DOMWrite/ReadStructuredClone are gone.
What I would do is to check if StructuredCloneHelper::Write/ReadFullySerializableObjects are good enough for you:

https://mxr.mozilla.org/mozilla-central/source/dom/base/StructuredCloneHelper.cpp#415

In case, extend them. They are already in used in any postMessage() methods and you can use them anywhere else you want.
I can either write this code or review it.

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Andrea Marchesini (:baku) from comment #3)
Thanks for the comments, I'd work on this.

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: WIP patch.

Hi Baku
I found nsIPrincipal is already handled in CustomRead/WriteHandler in ExportHelpers.cpp, via the tag SCTAG_REFLECTOR.

So I just added wrapReflectors: true in SystemAppProxy.jsm, but it still fails 

10-23 04:47:36.296: I/Gecko(8154): [Parent 8154] WARNING: NS_ENSURE_SUCCESS(rv, nullptr) failed with result 0x80570027: file /home/allstars/ssd/B2G_FlameKK/gecko/js/xpconnect/wrappers/WrapperFactory.cpp, line 274
10-23 04:47:36.296: E/GeckoConsole(8154): [JavaScript Error: "Error: Permission denied for <app://system.gaiamobile.org> to create wrapper for object of class UnnamedClass" {file: "app://system.gaiamobile.org/js/about_service_workers_proxy.js" line: 89}]

Can you help to provide some suggestions for me ?

Thanks

Comment 6

[Andrea Marchesini [:baku]]

> Can you help to provide some suggestions for me ?

There is not a relationship between wrapReflactors option and nsIPrincipal in our code. Actually in StructureCloneData we don't serialize nsIPrincipal at all. What we should do in order to support it is to call Read/WriteFullySerializableObjects in StructuredCloneData::CustomRead/WriteHandler.

Do you want me to do it or do you want to write a patch for it?

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Just discussed with khuey with this, he thinks even I make it successfully to pass nsIPrincipal, but Settings app will still have problems for reading it since it runs on content.

So in this bug I'll make it pass originNoSuffix in 
https://dxr.mozilla.org/mozilla-central/source/b2g/components/AboutServiceWorkers.jsm?case=true&from=AboutServiceWorkers.jsm#38

meanwhile I'll check baku's comment 6 and file another bug for fixing it.
Thanks

Comment 8

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch.

Hi Fernando
Could you review this simple fix for me?

Thanks

Comment 9

[Fernando Jiménez Moreno [:ferjm]]

I don't think this affects the test but it's worth checking that https://mxr.mozilla.org/mozilla-central/source/dom/workers/test/serviceworkers/test_aboutserviceworkers.html?force=1 still passes locally (it is disabled on tbpl for intermitent failures)

Comment 10

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

https://hg.mozilla.org/integration/b2g-inbound/rev/252fada28a344673d294c0dc80951d8b5c056d56
Bug 1196652 - OriginSuffix is shown in about:serviceworker on b2g. r=ferjm

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/252fada28a34