Closed Bug 676719 Opened 9 years ago Closed 7 years ago

Record URLs for plugin processes

Categories

(Core :: Plug-ins, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: marcia, Assigned: adw)

References

Details

Attachments

(1 obsolete file)

As we continue doing more investigation in the plugin realm, would be helpful to have URLs for these types of crashes. We are getting asked in bugs such as Bug 673433, for example.
This is a client-side bug. We'd need to add some code to the OOPP implementation to record this data.
Component: Socorro → Plug-ins
Product: Webtools → Core
QA Contact: socorro → plugins
There's a bug on this with discussion already...
Whiteboard: DUPEMe
No matter if in this bug or elsewhere, we really need to get something implemented for this, as from what I see the content crashes have the same problem, and we both want to be able to communicate to plugin vendors what URLs their problems happen on as well as find out what websites crash when using content process.
I couldn't find it in the list of bugs, so please feel free to dupe this bug to that bug and KaiRo and I will add ourselves to that bug.

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> There's a bug on this with discussion already...
We are being asked more and more for URLs to help us diagnose some of the harder crash issues. The bugs just end up sitting there - a number of our top Flash issues are like that. Per KaiRo's comments, won't we need this for the content crashes also? I know this is probably a difficult problem to solve.

Any way we can find the other bug or if the discussion happened elsewhere, get it pasted in this bug so we are all up to speed on the issues?
(In reply to Sheila Mooney from comment #5)
> Per KaiRo's comments, won't we need this for
> the content crashes also?

Apparently, we have some URLs for some content crashes. There seems to be a different problem at work why Fennec gets almost no URLs reported.

Still, we really need this more and more for plugins as we intensify our communications with plugin vendors and even vendors of applications running in plugins (think Farmville etc.) and so we really want something to move forward here.
OS: Mac OS X → All
Priority: -- → P1
Whiteboard: DUPEMe
Benjamin, could you point me to the back-end code related to plugin crash-handling?  I know in the front end we capture crash events to do things like prompting the user to send a crash report, and I presume the report could be annotated at that point, but this bug should probably be fixed in the back end, right?
Assignee: nobody → adw
Status: NEW → ASSIGNED
Backend code: http://hg.mozilla.org/mozilla-central/annotate/d042ad078f43/dom/plugins/ipc/PluginModuleParent.cpp#l446

But I really don't think that doing this in the backend would be either easier or better. The frontend knows which tab was focused at the time the crash occurs, which is probably the tab that "caused" the crash (not for certain, but close enough for stats work!). So I think doing this in the frontend is very reasonable.
Attached patch patch (obsolete) — Splinter Review
Thanks, Benjamin.  This adds a PluginContentURL key to the form data sent to the server.
Attachment #666054 - Flags: review?(benjamin)
Comment on attachment 666054 [details] [diff] [review]
patch

Looks good to me, but isn't CrashSubmit.jsm used from other places? Are you prepared to handle an "undefined" extraExtraKeyVals? ted should look this over. And it really needs a test. Since this is Firefox-specific chrome, we might need to add a test in a new location (the current locations in dom/plugins/test are generic).
Attachment #666054 - Flags: review?(ted.mielczarek)
Attachment #666054 - Flags: review?(benjamin)
Attachment #666054 - Flags: review+
Having pondered this, I think we are missing an important piece of UI: URLs have an opt-out checkbox, and this patch would submit them automatically without the user being explicitly aware of it. That's why I lumped this bug and bug 648675 together, since they both require UI changes and it may make sense to have a completely different UI (e.g. a popout).
Gavin thinks a checkbox would be OK.  I'll put it under the comment box and above the Send button in Jennifer's mockup in bug 648675 and do both bugs at the same time.
Comment on attachment 666054 [details] [diff] [review]
patch

The patch in bug 648675 covers this bug, too.
Attachment #666054 - Flags: review?(ted.mielczarek)
Is that patch going to add UI so people can opt out of reporting the URl while still reporting the crash, or so they are at least informed that when they send the report, we include the URL? We need to do that out of privacy reasons, as far as I'm informed (and we do that for browser crashes - we have a default-unchecked checkbox there for reporting the URL9.
(It's default-checked, FWIW, but we persist the user's choice.)
Comment on attachment 666054 [details] [diff] [review]
patch

Thought I marked this obsolete.
Attachment #666054 - Attachment is obsolete: true
This is fixed by bug 648675.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.