Need a way to create invisible, permanent docshells

RESOLVED FIXED in mozilla27

Status

()

defect
RESOLVED FIXED
9 years ago
2 years ago

People

(Reporter: Felipe, Assigned: gkrizsanits)

Tracking

({dev-doc-needed})

Trunk
mozilla27
x86
All
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 3 obsolete attachments)

Such feature would be useful to many add-ons, and historically the hidden window has been (mis-)used for this purpose. The main point is that sometimes a page would need to be loaded independent of the life-cycle of any particular browser window.
One of the Jetpack APIs would benefit from this, to avoid using the hidden window and doing workarounds on it.


One way to do this would be the ability to create more than 1 hidden window. Do we need a full window to make a docshell work? I don't know exactly what are the options here.  It would also be good to have it implemented in such a way that these docshells will live in the content process for when e10s lands.
My extension uses canvas.drawWindow to take a snapshot of an element, would that be a use case of this?
Matthew: is the element in an existing docshell (web page loaded into a tab, sidebar, chrome browser window, etc.)?  If so, then it wouldn't benefit from the functionality being requested here.

If, however, you need to load the page containing that element in a way that doesn't display the page for the user in order to take that snapshot, then this functionality would be very helpful to you.

Does that make sense?
I need to take a snapshot of a translucent panel that's overlaid onto the chrome.  The snapshot should maintain the translucency and not incorporate what's behind.

The reason for this is I have a "flip animation" that emulates something turning around, which necessitates scaling the element in the X-axis ...

In order to do this correctly I currently create a carbon copy of the element in a scratch IFrame that I also overlay into the browser chrome.

The current implementation works as far as it goes, but is really finicky, and I was wondering if this would make it smoother and remove the need to start inserting stuff into the chrome DOM.

Now that I think about it, I also have printing functionality which writes HTML into a scratch IFRAME which might benefit from this too.
So is what's wanted here basically the sort of setup TabChild::Init does?  We could maybe even skip on the widget bits if we don't care about layout inside the nsWebBrowser, though it sounds like at least some of the consumers might care about it?
bz: I'm unfamiliar with TabChild::Init, and reading through it just now didn't immediately clarify matters, but to address your question, I think that some of the consumers of this API would care about layout.

One hypothetical example is an addon that updates the snapshots of your popular sites on the New Tab page. I could also imagine an addon that uses the Google Maps API (which, the last time I checked, requires you to load a Google Maps page) to generate map images.

I'm sure I used to know of better, and non-hypothetical examples, but somehow I can't think of them right now.  And the very real use case from bug 820953 probably doesn't care about layout.  So if it's easier to get things working without layout first, then I would elect that option.

Also setting needsinfo on Felipe in case he has better examples.
Flags: needinfo?(felipc)
> I'm unfamiliar with TabChild::Init

It's what we use to set up the docshells in the child process in e10s, fwiw.

I'm hoping to get to this bug in the near future, but at this point that might be after the new year.  :(
Clearing ignored needinfo request.
Flags: needinfo?(felipc)
bz pointed out to me that TabChild::Init does almost exactly what we want already. That is, it creates a PuppetWidget, which is a widget that has no associated OS window, and associates that with the doc shell.

The only problem for our use case is that we have no tab child either, and PuppetWidget is currently unable to handle NULL tab childs. I've opened bug 846881 to address this problem.

The second problem is that widgets are currently not scriptable, so there's no way to create a PuppetWidget from within JavaScript. This is easily solved by exposing a function that replicates what TabChild::Init, and returns some usable object (presumably a docshell).
Depends on: 846881
QA Contact: ejpbruel
Depends on: 846906
Eddy here is simplified test case to reproduce crash:
https://gist.github.com/Gozala/6033211
Flags: needinfo?(ejpbruel)
bz wha would be a right way to destroy this windowless docshells ? So that they loaded docs and js within them will be GC-ed ?

Only think I can think of is:

docShell.contentViewer.close();
docShell.contentViewer.destroy();
docShell = null;

But I'm not sure if that's a right way to go about or if some of this is redundant.
Flags: needinfo?(bzbarsky)
I would think that should happen automatically, no?  As in, once you drop the reference to the nsIWebBrowser, it should get GCed.
Flags: needinfo?(bzbarsky)
But when does document in it is going to be unloaded ? Or what if something holds reference to an object from the document in it ?

Having a way to destroy it manually would be useful.
Assignee: nobody → ejpbruel
Flags: needinfo?(ejpbruel)
QA Contact: ejpbruel
> But when does document in it is going to be unloaded ?

When the nsIWebBrowser is GCed.

> Or what if something holds reference to an object from the document in it ?

That would keep the object alive, even if you manually call destroy() on the docshell, obviously.

A bigger concern is what happens if you have a cycle through the objects in the loaded document that keep the nsIWebBrowser alive...

> Having a way to destroy it manually would be useful.

In that case, just destroy() on the nsIWebBrowser should be what you want.
Hey Eddy, I just want to point out that we're still using iframe workarounds 
since we're not able to switch to this because it causes crashes as pointed out earlier. Do you by any chance have some update on this ?
Flags: needinfo?(ejpbruel)
It's on my to do list for when I get back from the work week. If I have some spare time, I could even look at it during the work week, but no promises there.
Flags: needinfo?(ejpbruel)
Assignee

Comment 17

6 years ago
(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #10)
> Eddy here is simplified test case to reproduce crash:
> https://gist.github.com/Gozala/6033211

I had some time to look into this and Eddy said he is busy with other bugs right now, so here it is.

scratchpad code:

let Ci = Components.interfaces;
let Cc = Components.classes;
 
let addonPrincipal = Cc["@mozilla.org/systemprincipal;1"].
createInstance(Ci.nsIPrincipal);
let docShell = Cc["@mozilla.org/appshell/appShellService;1"].
getService(Ci.nsIAppShellService).
createWindowlessBrowser().
QueryInterface(Ci.nsIInterfaceRequestor).
getInterface(Ci.nsIDocShell);
 
docShell.createAboutBlankContentViewer(addonPrincipal);
 
let win = docShell.contentViewer.DOMDocument.defaultView;
win.location = "data:application/vnd.mozilla.xul+xml;charset=utf-8,<window/>";

It hits an assertion in nsDocumentOpenInfo::TryContentListener because the content viewer creation fails. Problem is that the channel has null principal and that cannot load xul. Since this all tells me very little, I ask bz and hope he knows what is going on, and what should be fixed...

call stack:

>	xul.dll!MayUseXULXBL(nsIChannel * aChannel=0x1f9e01f0) Line 128	C++
 	xul.dll!nsContentDLF::CreateInstance(const char * aCommand=0x06973718, nsIChannel * aChannel=0x1f9e01f0, nsILoadGroup * aLoadGroup=0x1f1202b8, const char * aContentType=0x1f9e02c0, nsISupports * aContainer=0x19b28c98, nsISupports * aExtraInfo=0x00000000, nsIStreamListener * * aDocListener=0x1f9e05ac, nsIContentViewer * * aDocViewer=0x0034ef3c) Line 236	C++
 	xul.dll!nsDocShell::NewContentViewerObj(const char * aContentType=0x1f9e02c0, nsIRequest * request=0x1f9e01f0, nsILoadGroup * aLoadGroup=0x1f1202b8, nsIStreamListener * * aContentHandler=0x1f9e05ac, nsIContentViewer * * aViewer=0x0034ef3c) Line 8163	C++
 	xul.dll!nsDocShell::CreateContentViewer(const char * aContentType=0x1f9e02c0, nsIRequest * request=0x1f9e01f0, nsIStreamListener * * aContentHandler=0x1f9e05ac) Line 7960	C++
 	xul.dll!nsDSURIContentListener::DoContent(const char * aContentType=0x1f9e02c0, bool aIsContentPreferred=false, nsIRequest * request=0x1f9e01f0, nsIStreamListener * * aContentHandler=0x1f9e05ac, bool * aAbortProcess=0x0034f02a) Line 123	C++
 	xul.dll!nsDocumentOpenInfo::TryContentListener(nsIURIContentListener * aListener=0x1f120438, nsIChannel * aChannel=0x1f9e01f0) Line 681	C++
 	xul.dll!nsDocumentOpenInfo::DispatchContent(nsIRequest * request=0x1f9e01f0, nsISupports * aCtxt=0x00000000) Line 382	C++
 	xul.dll!nsDocumentOpenInfo::OnStartRequest(nsIRequest * request=0x1f9e01f0, nsISupports * aCtxt=0x00000000) Line 258	C++
 	xul.dll!nsBaseChannel::OnStartRequest(nsIRequest * request=0x1f9e8ba8, nsISupports * ctxt=0x00000000) Line 720	C++
 	xul.dll!nsInputStreamPump::OnStateStart() Line 463	C++
 	xul.dll!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream * stream=0x1f9e0720) Line 388	C++
 	xul.dll!nsInputStreamReadyEvent::Run() Line 83	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait=false, bool * result=0x0034f366) Line 622	C++
 	xul.dll!NS_ProcessNextEvent(nsIThread * thread=0x00703138, bool mayWait=false) Line 238	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate * aDelegate=0x006fd220) Line 81	C++
 	xul.dll!MessageLoop::RunInternal() Line 221	C++
 	xul.dll!MessageLoop::RunHandler() Line 214	C++
 	xul.dll!MessageLoop::Run() Line 188	C++
 	xul.dll!nsBaseAppShell::Run() Line 163	C++
Flags: needinfo?(bzbarsky)
Well, the channel has null principal because the URI is a data: URI.  And it can't inherit one from the parent window or whatnot because there is no parent to inherit from (and in any case, inheriting the system principal into a non-chrome docshell is disallowed for security reasons).

If you must load XUL from data: URIs in a disconnected window from script, we'll need some new API...  Is the data: URI a hard requirement?
Flags: needinfo?(bzbarsky)
Assignee

Updated

6 years ago
Flags: needinfo?(rFobic)
(In reply to Boris Zbarsky [:bz] from comment #18)
> Well, the channel has null principal because the URI is a data: URI.  And it
> can't inherit one from the parent window or whatnot because there is no
> parent to inherit from (and in any case, inheriting the system principal
> into a non-chrome docshell is disallowed for security reasons).
>

I see, I assumed that following line would make the trick of providing custom principle:

docShell.createAboutBlankContentViewer(addonPrincipal);


> 
> If you must load XUL from data: URIs in a disconnected window from script,
> we'll need some new API...  Is the data: URI a hard requirement?
>

I don't think data URI is a requirement, it just a simplest option that worked
for us when using using iframes in the hidden window.

What we really need is a way to create a docshell with a custom principal. We also need
to be able to create iframes there with which we'll be able to swap frame loaders, now
that imposes dependency on XUL documents.
Flags: needinfo?(rFobic)
> I see, I assumed that following line would make the trick of providing custom principle:

Hmm.  So it should.  Have you stepped through the loadURI call to see why it's not being inherited?

> now that imposes dependency on XUL documents.

Does it?  It imposes a dependency on xul:iframe elements, but you can create those in an HTML document, I suspect.
Assignee

Comment 21

6 years ago
(In reply to Boris Zbarsky [:bz] from comment #20)
> Hmm.  So it should.  Have you stepped through the loadURI call to see why
> it's not being inherited?

In nsDocShell::GetInheritedPrincipal we have a typeContent as the mItemType that prevents us from inheriting the system principal it seems. And we have an about::blank document to start with if that matters... I guess if we want chrome privileges we should not start with about::blank document, but with something that is already chrome, or am I wrong?

> Does it?  It imposes a dependency on xul:iframe elements, but you can create
> those in an HTML document, I suspect.

I hope we can get rid of that swap frame loader magic from the SDK, but in any case we need here a chrome document for now, xul or HTML that might not be so crucial then.
> we have a typeContent as the mItemType that prevents us from inheriting the system
> principal it seems.

Sure.  But I assumed you were passing a non-system principal to createAboutBlankContentViewer()?  If you're passing a system principal, then yeah, that won't get inherited.

> I guess if we want chrome privileges we should not start with about::blank document, but
> with something that is already chrome

If you want chrome privileges, you shouldn't have a typeContent docshell, imo.  

How about we add an argument to createWindowlessBrowser that indicates whether you want a typeContent or typeChrome docshell?
Assignee

Comment 23

6 years ago
(In reply to Boris Zbarsky [:bz] from comment #22)
> Sure.  But I assumed you were passing a non-system principal to
> createAboutBlankContentViewer()?  If you're passing a system principal, then
> yeah, that won't get inherited.

Ah yeah, the addonPrincipal var name was a bit misleading in the original example, but it's really just this: let addonPrincipal = Cc["@mozilla.org/systemprincipal;1"].

> How about we add an argument to createWindowlessBrowser that indicates
> whether you want a typeContent or typeChrome docshell?

Sounds good to me. Eddy, do you want to implement this (and if so when?) or shall I take a look at it?
Flags: needinfo?(ejpbruel)
Assignee

Comment 24

6 years ago
Another exception... this time from ShadowLayerForwarder::BeginTransaction, HasShadowManager() returns null. Looks like it's some kind of paint even that our puppet widget failed to avoid... How can I prevent this?

 	xul.dll!mozilla::layers::ShadowLayerForwarder::BeginTransaction(const nsIntRect & aTargetBounds={...}, mozilla::ScreenRotation aRotation=ROTATION_0, const nsIntRect & aClientBounds={...}, unsigned int aOrientation=0x00000004) Line 202	C++
 	xul.dll!mozilla::layers::ClientLayerManager::BeginTransactionWithTarget(gfxContext * aTarget=0x00000000) Line 138	C++
 	xul.dll!mozilla::layers::ClientLayerManager::BeginTransaction() Line 156	C++
 	xul.dll!PresShell::Paint(nsView * aViewToPaint=0x192b0bf8, const nsRegion & aDirtyRegion={...}, unsigned int aFlags=0x00000001) Line 5603	C++
 	xul.dll!nsViewManager::ProcessPendingUpdatesForView(nsView * aView=0x192b0bf8, bool aFlushDirtyRegion=true) Line 411	C++
 	xul.dll!nsViewManager::ProcessPendingUpdates() Line 1041	C++
 	xul.dll!nsRefreshDriver::Tick(__int64 aNowEpoch=0x0004e6c04a8d1602, mozilla::TimeStamp aNowTime={...}) Line 1206	C++
 	xul.dll!mozilla::RefreshDriverTimer::TickDriver(nsRefreshDriver * driver=0x196d6458, __int64 jsnow=0x0004e6c04a8d1602, mozilla::TimeStamp now={...}) Line 167	C++
 	xul.dll!mozilla::RefreshDriverTimer::Tick() Line 158	C++
 	xul.dll!mozilla::RefreshDriverTimer::TimerTick(nsITimer * aTimer=0x1017e050, void * aClosure=0x1017dfa8) Line 184	C++
 	xul.dll!nsTimerImpl::Fire() Line 546	C++
 	xul.dll!nsTimerEvent::Run() Line 632	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait=false, bool * result=0x002ef24a) Line 622	C++
 	xul.dll!NS_ProcessNextEvent(nsIThread * thread=0x00bed748, bool mayWait=false) Line 238	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate * aDelegate=0x001aa8e0) Line 81	C++
Flags: needinfo?(bzbarsky)
I don't know.  Timothy, do you?
Flags: needinfo?(bzbarsky) → needinfo?(tnikkel)
PuppetWidgets still need to paint in general. What changes were made to try to make it not paint? Sorry if this has an obvious answer, I only skimmed all of the previous comments on this bug.
Flags: needinfo?(tnikkel)
Or are you asking how to avoid the abort_if_false?
Flags: needinfo?(gkrizsanits)
This puppetwidget is just there to make layout kinda happy.  It definitely doesn't need to paint.  But I'm not sure we ever really turned it off.
So we want a way to cut off all painting related activity at the source with as little fuss as possible then?
Flags: needinfo?(gkrizsanits) → needinfo?(tnikkel)
So perhaps if we can mark the puppet widget somehow (an attribute on some content node maybe? but then how to get that to the widget) the view manager code could check that "don't bother painting" flag on the widget before any kind of painting related activity. Or maybe the view manager could ask the presshell to look at some content node for this check.
Flags: needinfo?(tnikkel)
This puppet widget is set up directly by C++ code we control, and never needs to paint, so the marking part is easy.  There is no content node involved, though.  But of course we have a docshell we could set flags on or whatnot.
Oh then perfect. Once the widget is marked it's probably just a matter of adding a few if's in nsViewManager.cpp.
Assignee

Updated

6 years ago
Assignee: ejpbruel → gkrizsanits
Flags: needinfo?(ejpbruel)
Assignee

Comment 34

6 years ago
I guess this is the way to set the docshell in chrome mode... It seems to work... am I close?
Attachment #809912 - Flags: feedback?(bzbarsky)
Assignee

Comment 35

6 years ago
I'm not sure who should I flag for a feedback here... Feel free to pass the ball to Timothy. So I tried flagging the widget, but that would mean I have to flag any subsequent widget related to this docshell probably... so I ended up with this version instead, however, getting to the docshell from nsViewManager is kind of too complicated, and I'm not sure how hot is this code... Is there an easier way? Do I need these null checks or those should be only asserts?

Also I don't know how to write a proper test for this. In the example we do win.location = ... , is there a reliable way to get a notification when the new document is loaded?
Attachment #809919 - Flags: feedback?(bzbarsky)
Comment on attachment 809912 [details] [diff] [review]
part1: chrome option for the invisible docshell

Looks right, but what's with the added SetInvisible() call?

Also, you could make the argument optional, so the default for no args would be non-chrome.
Attachment #809912 - Flags: feedback?(bzbarsky) → feedback+
Attachment #809919 - Flags: feedback?(bzbarsky) → feedback?(tnikkel)
> Is there a reliable way to get a notification when the new document is loaded?

You can use a web progress listener, right?
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #35)
> So I tried flagging the widget, but that would mean I have
> to flag any subsequent widget related to this docshell probably... so I
> ended up with this version instead, however, getting to the docshell from
> nsViewManager is kind of too complicated, and I'm not sure how hot is this
> code... 

How hard is it to flag the widget from the docshell? I think I would prefer that, unless it's overly complicated.
Assignee

Comment 39

6 years ago
(In reply to Boris Zbarsky [:bz] from comment #36)
> Comment on attachment 809912 [details] [diff] [review]
> part1: chrome option for the invisible docshell
> 
> Looks right, but what's with the added SetInvisible() call?

whoops... I split the changes into two patches quickly, and that part should belong to part2... I will fix it.

> 
> Also, you could make the argument optional, so the default for no args would
> be non-chrome.

Hmmm... how can I do that in xpidl?
  nsIWebNavigation createWindowlessBrowser([optional] in bool aIsChrome);
Assignee

Comment 41

6 years ago
(In reply to Timothy Nikkel (:tn) from comment #38)
> (In reply to Gabor Krizsanits [:krizsa :gabor] from comment #35)
> 
> How hard is it to flag the widget from the docshell? I think I would prefer
> that, unless it's overly complicated.

At widget creation time I don't think that can be done. What is the problem with the current one? The best I could come up with is doing it in nsBaseWidget::SetWidgetListener. There I could fetch a reference to the docshell from the widget listener with the same dance like I did it in the current version and set the flag on the widget as well. Would that be any better? Your concern is performance or something else?
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #41)
> (In reply to Timothy Nikkel (:tn) from comment #38)
> > (In reply to Gabor Krizsanits [:krizsa :gabor] from comment #35)
> > 
> > How hard is it to flag the widget from the docshell? I think I would prefer
> > that, unless it's overly complicated.
> 
> At widget creation time I don't think that can be done. What is the problem
> with the current one? The best I could come up with is doing it in
> nsBaseWidget::SetWidgetListener. There I could fetch a reference to the
> docshell from the widget listener with the same dance like I did it in the
> current version and set the flag on the widget as well. Would that be any
> better? Your concern is performance or something else?

The most natural place to store something that says "you don't have to paint this" is in the widget, and the view code is mostly self contained with external references to frames, presshells and widgets, but not to content nodes or docshells. It would be nice to keep view as an intermediary between widgets and layout without talking to other things. I'll think about if there is a better way to do this.
Assignee

Comment 43

6 years ago
(In reply to Timothy Nikkel (:tn) from comment #42)
> The most natural place to store something that says "you don't have to paint
> this" is in the widget, and the view code is mostly self contained with
> external references to frames, presshells and widgets, but not to content
> nodes or docshells. It would be nice to keep view as an intermediary between
> widgets and layout without talking to other things. I'll think about if
> there is a better way to do this.

Oh, I understand. Well, I would argue that since the information we store is more like: "don't paint any of these / anything at all" maybe we could store it one level higher, on the presshell. It already has IsPaintingSuppressed... Unless you can come up with something better ofc, I just got stuck with the widget approach.
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #43)
> (In reply to Timothy Nikkel (:tn) from comment #42)
> > The most natural place to store something that says "you don't have to paint
> > this" is in the widget, and the view code is mostly self contained with
> > external references to frames, presshells and widgets, but not to content
> > nodes or docshells. It would be nice to keep view as an intermediary between
> > widgets and layout without talking to other things. I'll think about if
> > there is a better way to do this.
> 
> Oh, I understand. Well, I would argue that since the information we store is
> more like: "don't paint any of these / anything at all" maybe we could store
> it one level higher, on the presshell. It already has
> IsPaintingSuppressed... Unless you can come up with something better ofc, I
> just got stuck with the widget approach.

Presshell would be a better place than the docshell, do you have an approach that works okay for that?
Assignee

Comment 45

6 years ago
Something like this. I would still keep the invisible flag on the docshell, this makes it easy to set all presshell to non painting mode at creation time, and I find it a usefull info to keep around on it anyway... (invisble docshells are quite special, there is a big chance that sooner or later we need some exception cases for them at some other places too, not just at painting... it's kind of a whack-a-mole game unfortunately)
Attachment #809919 - Attachment is obsolete: true
Attachment #809919 - Flags: feedback?(tnikkel)
Attachment #811964 - Flags: feedback?(tnikkel)
Assignee

Comment 46

6 years ago
Eh... forgot to change the iid of nsIPresShell, and move the docshell->SetInvisible(true); line from patch part1 to this patch too (nsAppShellService::CreateWindowlessBrowser). But I guess the concept is clear anyway, so I will fix these and add test too if you find this version good enough to continue...
Comment on attachment 811964 [details] [diff] [review]
part2: no paint for invisible docshell. v2

This seems good.

There might be better places (and more) places to put ifs in the view code to avoid even more useless work, but the approach looks good.
Attachment #811964 - Flags: feedback?(tnikkel) → feedback+
Assignee

Comment 48

6 years ago
Attachment #809912 - Attachment is obsolete: true
Attachment #814429 - Flags: review?(bzbarsky)
Assignee

Comment 49

6 years ago
Attachment #811964 - Attachment is obsolete: true
Attachment #814431 - Flags: review?(tnikkel)
Comment on attachment 814429 [details] [diff] [review]
part1: chrome option for the invisible docshell. v2

r=me
Attachment #814429 - Flags: review?(bzbarsky) → review+
Comment on attachment 814431 [details] [diff] [review]
part2: no paint for invisible docshell. v3

This looks good, thank you. Sorry for the delay.

Do you think we should change PresShell::ShouldIgnoreInvalidation to return true if mIsNeverPainting is true instead of making the ShouldIgnoreInvalidation changes in the view manager? Functionally the same, but perhaps a little nicer.
Attachment #814431 - Flags: review?(tnikkel) → review+
Comment on attachment 814431 [details] [diff] [review]
part2: no paint for invisible docshell. v3

>diff --git a/docshell/test/chrome/Makefile.in b/docshell/test/chrome/Makefile.in
>+		test_bug565388.xul \

You've got this random hunk in here though.
Assignee

Comment 53

6 years ago
(In reply to Timothy Nikkel (:tn) from comment #51)
> Comment on attachment 814431 [details] [diff] [review]
> Do you think we should change PresShell::ShouldIgnoreInvalidation to return
> true if mIsNeverPainting is true instead of making the
> ShouldIgnoreInvalidation changes in the view manager? Functionally the same,
> but perhaps a little nicer.

Yes, that's a good idea I will do that.

> >diff --git a/docshell/test/chrome/Makefile.in b/docshell/test/chrome/Makefile.in
> >+		test_bug565388.xul \
> 
> You've got this random hunk in here though.

Whoops... that random hunk supposed to be a test that I forgot to add to the patch. And it seems like we have to add tests differently these days... Anyway I just add the test as another patch. I would ask Boris to take a look at the test but since he is on vacation could you review the test for me? It does not do much just follows the steps that caused a crash and use progress listener to see if the navigation has finished successfully.
Attachment #816471 - Flags: review?(tnikkel)
Comment on attachment 816471 [details] [diff] [review]
part3: test. v1

Looks fine to me.
Attachment #816471 - Flags: review?(tnikkel) → review+

Comment 56

6 years ago
Great work!  Can you please post a summary on how to use this API to dev.platform?

Thanks!
https://hg.mozilla.org/mozilla-central/rev/98fe3c07b042
https://hg.mozilla.org/mozilla-central/rev/9373baf8aec2
https://hg.mozilla.org/mozilla-central/rev/9027ca2649fc
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Assignee

Comment 58

6 years ago
Irakli: When you have time for it to try the new version do let me know if you run into any other issues.
Flags: needinfo?(rFobic)

Updated

6 years ago
Keywords: dev-doc-needed
I've being discussed issues in the bug 946015, so clearing needinfo here.
Flags: needinfo?(rFobic)
Comment on attachment 778054 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1125

Clearing feedback? for this patch because I don't think it's relevant anymore. Feel free to reset it if I'm wrong.
Attachment #778054 - Flags: feedback?(ejpbruel)
Reopening this issues as issue in the following comment is still not fixed and blocks us from using it
https://bugzilla.mozilla.org/show_bug.cgi?id=946015#c5
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Probably best to open a new bug for that issue. Re-opening a year+ old bug is going to lead to confusion with target milestone and tracking regressions from any new work done in this same bug.
Indeed.
Status: REOPENED → RESOLVED
Closed: 6 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.