980750 - Register content handler after the child process is forked from Nuwa

Status: RESOLVED WONTFIX

(Core :: DOM: Content Processes, defect)

Description

[Cervantes Yu [:cyu] [:cervantes]]

Forked from https://bugzilla.mozilla.org/show_bug.cgi?id=972816#c6

We need to delay content handlers after a content process is forked from Nuwa.

Comment 1

[Cervantes Yu [:cyu] [:cervantes]]

Nominate 1.3? since this can cause regression of content handlers.

Comment 2

[Jason Smith [:jsmith]]

(In reply to Cervantes Yu from comment #1)
> Nominate 1.3? since this can cause regression of content handlers.

Can you be more specific? When this could happen on the web? What's the user impact?

Comment 3

[Cervantes Yu [:cyu] [:cervantes]]

(In reply to Jason Smith [:jsmith] from comment #2)
> (In reply to Cervantes Yu from comment #1)
> > Nominate 1.3? since this can cause regression of content handlers.
> 
> Can you be more specific? When this could happen on the web? What's the user
> impact?

As Fabrice said in https://bugzilla.mozilla.org/show_bug.cgi?id=972816#c6 the new content type might not be handled correctly a new app for that content type is installed.

Comment 4

[Jason Smith [:jsmith]]

Fabrice - Can you weigh in here on the severity of this bug?

Comment 5

[Preeti Raghunath(:Preeti)]

Fabrice,

Please help with user impact analysis.

Comment 6

[[:fabrice] Fabrice Desré]

The impact is that if you install an app that can be a content handler, we won't register this content handler until the next restart of the phone. So yes, that's at least annoying. But I have no idea how many apps in the wild make use of this feature.

Cervantes, if I move this registration to TabChild::PreloadSlowThings() we should be good right? (note: we should rename that to PreloadAfterFork()...)

Comment 7

[Cervantes Yu [:cyu] [:cervantes]]

(In reply to Fabrice Desré [:fabrice] from comment #6)
> Cervantes, if I move this registration to TabChild::PreloadSlowThings() we
> should be good right? (note: we should rename that to PreloadAfterFork()...)

I should be, but we might skip preloading slow things for Tarako to reduce the memory consumption of the preallocated process. You may add this registration to InitOnContentProcessCreated(): http://hg.mozilla.org/mozilla-central/file/428e5462fe29/dom/ipc/ContentChild.cpp#l367. This method is added for child process initialization that must be done for Nuwa-enabled builds. It's also working as expected for non-Nuwa builds.

Comment 8

[[:fabrice] Fabrice Desré]

Ok, I'll add it in InitOnContentProcessCreated() then. Note running PreloadSlowThings() will kill our startup time so we should do that only as a desperate measure.

Comment 9

[Preeti Raghunath(:Preeti)]

Adam

Please add comments from contents from Marketplace.

Comment 10

[Adam Rogers (:arog)]

I've reviewed the apps by downloads (list generated 3/7/2014) and it doesn't appear to me that there are any applications that would fall in this category in the top 50 applications. For that reason, I wouldn't block the release on this issue.

I suggest moving this to the priority backlog.

Comment 11

[Preeti Raghunath(:Preeti)]

Moving to backlog per comment 10

Comment 12

[Jason Smith [:jsmith]]

(In reply to Adam Rogers (:arog) from comment #10)
> I've reviewed the apps by downloads (list generated 3/7/2014) and it doesn't
> appear to me that there are any applications that would fall in this
> category in the top 50 applications. For that reason, I wouldn't block the
> release on this issue.

We probably need look at this from a contextual perspective from a script, not a top 50 list. There's more than 50 apps even in the partner app list alone. You are looking for apps such as:

* Third-Party PDF Viewers
* Third-Party Music Players
* Third-Party Video Players
* etc.

This can scripted by looking for a pick activity with a filter containing a mime type.

However - Karen mentioned in person that introducing content handlers for apps is frowned upon, as other apps commonly want to handle content directly within the app itself. On that regard, I think we're okay to live without this.

I'm going to leave needinfo though on Lisa & Harald here though to double check if they know of any apps that are currently acting as content handlers.

Comment 13

[Lisa Brewster [:adora]]

My lack of response means I'm not aware of any apps that make use of this.  Impact is minor, as long as it's documented so developers know to advise their users to reboot.

Comment 14

[Jan Beich]

Nuwa is gone after bug 1284674.