Closed
Bug 1445659
Opened 7 years ago
Closed 6 years ago
Implement a minimal form of Abstract Browsing Contexts
Categories
(Core :: DOM: Core & HTML, enhancement, P2)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: farre, Assigned: farre)
References
(Blocks 1 open bug)
Details
Attachments
(7 files, 22 obsolete files)
1.26 KB,
application/zip
|
Details | |
5.56 KB,
patch
|
farre
:
review+
|
Details | Diff | Splinter Review |
9.70 KB,
patch
|
Details | Diff | Splinter Review | |
10.38 KB,
patch
|
farre
:
review+
|
Details | Diff | Splinter Review |
4.36 KB,
patch
|
nika
:
review+
|
Details | Diff | Splinter Review |
6.84 KB,
patch
|
Details | Diff | Splinter Review | |
38.03 KB,
patch
|
farre
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → afarre
Blocks: browsingcontext
Assignee | ||
Comment 1•7 years ago
|
||
Assignee | ||
Comment 2•7 years ago
|
||
Assignee | ||
Comment 3•7 years ago
|
||
Assignee | ||
Comment 4•7 years ago
|
||
Comment on attachment 8959482 [details] [diff] [review]
0001-Bug-1445659-Create-AbstractBrowsingContexts-in-paren.patch
Finally got something together. This creates an AbstractBrowsingContext in both TabParent and TabChild, like we discussed. Only TabChild populates the docshell of the ABC, and attaching + detaching hangs off of the end of nsDocShell::InternalLoad, because that's where we converge with different kinds of loads.
The 0002-patch exposes a function getAbstractBrowsingContext on Window that I've used for testing, and the zip attached contains such a test. If you unzip that and run two servers locally on different ports, then you can see ABC's being built by calling 'postMessage('get', '*') in the debug console.
Attachment #8959482 -
Flags: feedback?(peterv)
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Andreas Farre [:farre] from comment #4)
> Comment on attachment 8959482 [details] [diff] [review]
> 0001-Bug-1445659-Create-AbstractBrowsingContexts-in-paren.patch
>
> Finally got something together.
And I should say that it only stores the bare minimum on the ABC's.
Assignee | ||
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Comment 6•7 years ago
|
||
Assignee | ||
Comment 7•7 years ago
|
||
There are some ipc errors, due to the silly broadcasting in TabParent. Still very much work in progress.
Attachment #8959482 -
Attachment is obsolete: true
Attachment #8959483 -
Attachment is obsolete: true
Attachment #8959482 -
Flags: feedback?(peterv)
Flags: needinfo?(peterv)
Assignee | ||
Comment 8•7 years ago
|
||
Moved ABCs to docshell, where they should've been all along. Also fixed detaching, but I'm not really happy, since we have the map of all ABCs in a TabChild we can never just deref a node and assume that the entire subtree is detached.
Attachment #8968452 -
Attachment is obsolete: true
Attachment #8968453 -
Attachment is obsolete: true
Assignee | ||
Comment 9•7 years ago
|
||
This removes AbstractBrowsingContextDictionary in favour of having a StaticAutoPtr<nsDataHashTable<uint64_t, AbstractBrowsingContext>> of all available contexts, and a StaticAutoPtr<LinkedList<RefPtr<AbstractBrowsingContext>> of all root contexts and static ::Get + ::GetOrCreate.
Attach + Detach of ABCs sort of works, but breaks for session history and some other cases. It is in no way clear when to either attach or detach a child context to a parent or a root to the list of roots, especially since it is required that the PBrowser is present.
Attachment #8973695 -
Attachment is obsolete: true
Updated•7 years ago
|
Summary: Create AbstractBrowsingContexts in parent/child → Implement a minimal form of Abstract Browsing Contexts
Assignee | ||
Comment 10•7 years ago
|
||
Move Attach/Detach from PBrowser to PContent. Make child abc's refcounted.
Attachment #8974470 -
Attachment is obsolete: true
Assignee | ||
Comment 11•7 years ago
|
||
Assignee | ||
Comment 12•7 years ago
|
||
Comment 13•7 years ago
|
||
So I've been thinking a bit more about the AbstractBrowsingContext name, and despite the 'ABC' being really cute, I'm starting to think we should just call it a BrowsingContext.
The original reason why we wanted not to use the name 'BrowsingContext' was because it seemed more appropriate as a name for nsDocShell. However, I think ABCs match the spec definition of a BrowsingContext, as it persists across browser navigations. The nsDocShell acts more as a "Document Loader" which handles loading a sequence of related documents within an individual process.
Using the BrowsingContext name will likely require less explanation to people familiar with the standard, and is also a shorter name.
How do you feel about that change :farre & :bz?
Flags: needinfo?(bzbarsky)
Flags: needinfo?(afarre)
Comment 14•7 years ago
|
||
In the new world where docshell is not persistent across navigations, makes sense to me.
Flags: needinfo?(bzbarsky)
Comment 15•7 years ago
|
||
Wasn't the idea that AbstractBrowserContext would then have concrete, per process BrowsingContexts (== DocShells). From a process point of view, DocShell is BrowsingContext. And things like
"document does not have a browsing context" in the spec refers to something closer to DocShell.
And per spec BrowsingContext is the thing which loads stuff.
"A browsing context is an environment in which Document objects are presented to the user."
ABC which doesn't have docshell attached to it in the current process doesn't have any document objects presented to the user.
But whatever works the best. I have my opinion, but it isn't strong.
Assignee | ||
Comment 16•7 years ago
|
||
Assignee | ||
Comment 17•7 years ago
|
||
git mv AbstractBrowsingContext.* -> BrowsingContext.*
I'm in total agreement.
Attachment #8976982 -
Attachment is obsolete: true
Flags: needinfo?(afarre)
Assignee | ||
Comment 18•7 years ago
|
||
Attachment #8982831 -
Attachment is obsolete: true
Attachment #8983525 -
Flags: review?(nfroyd)
Comment 19•7 years ago
|
||
Comment on attachment 8983525 [details] [diff] [review]
0001-Bug-1445659-Make-it-possible-to-store-RefPtr-T-in-Au.patch
Review of attachment 8983525 [details] [diff] [review]:
-----------------------------------------------------------------
Good catch. I have no problem with the code, but please add some tests, too. :)
::: mfbt/LinkedList.h
@@ +98,5 @@
> // to another, no enter or exit calls happen since the elements still belong
> // to a list.
> static void enterList(LinkedListElement<T>* elt) {}
> static void exitList(LinkedListElement<T>* elt) {}
> + static void cleanElement(LinkedListElement<T>* elt) { delete elt; }
Please provide some documentation for this new method, since the previous documentation doesn't cover this method.
Attachment #8983525 -
Flags: review?(nfroyd) → review-
Assignee | ||
Comment 20•7 years ago
|
||
Boris, me and Peter have been discussing this patch while I've been implementing it, so I was wondering if you would be ok with him reviewing it? Otherwise I'd be perfectly happy for you or Olli to do it, whatever makes it easier.
It has recently started to look landable, and the new BC class actually gets some use by moving name from nsDocShell to BrowsingContext.
Attachment #8982832 -
Attachment is obsolete: true
Flags: needinfo?(bzbarsky)
Assignee | ||
Comment 21•7 years ago
|
||
Thanks for the catch, I had that documentation uncommitted locally.
I added two _very_ simple tests. I didn't reset the r? flag since I'm a bit hesitant about those two tests being enough (and the fact that cppunittests refuse to run for me locally).
Will return to it on thursday after tomorrows holiday.
Attachment #8983525 -
Attachment is obsolete: true
Flags: needinfo?(nfroyd)
Assignee | ||
Updated•7 years ago
|
Attachment #8983526 -
Flags: review?(peterv)
Updated•7 years ago
|
Blocks: rm-docshell-tree-item
Assignee | ||
Comment 23•7 years ago
|
||
Test cases added to the AutoCleanLinkedList patch. Moved to gtest and found and fixed a bug! In the previous version delete was called on LinkedListElement<T>*, which happens to have a non-virtual destructor. The solution is to call delete on LinkedListElement<T>->asT(), similarly to how the other adapter methods do.
Attachment #8983543 -
Attachment is obsolete: true
Flags: needinfo?(nfroyd)
Attachment #8984076 -
Flags: review?(nfroyd)
Comment 24•7 years ago
|
||
Comment on attachment 8984076 [details] [diff] [review]
0001-Bug-1445659-Make-it-possible-to-store-RefPtr-T-in-Au.patch
Review of attachment 8984076 [details] [diff] [review]:
-----------------------------------------------------------------
Thank you for writing the tests!
::: mfbt/tests/gtest/TestLinkedList.cpp
@@ +23,5 @@
> + {
> + EXPECT_TRUE(!*mResult);
> + }
> +
> + virtual ~PtrClass() {
Does this really need to be virtual? Or is this to work around some compiler complaint? I think you could just make `PtrClass` `final`, and then the destructor doesn't need to be virtual.
Oh, bother, you inherit below. OK, never mind!
@@ +65,5 @@
> +{
> +public:
> + int mCount;
> + void AddRef() { mCount++; }
> + void Release() { mCount--; }
It is weird to see a Release() method that doesn't destroy the class, but OK!
@@ +71,5 @@
> + CountedClass()
> + : mCount(0)
> + {
> + }
> + virtual ~CountedClass() { EXPECT_TRUE(mCount == 0); }
Same reasoning applies here, with respect to declaring the class `final` and the destructor non-virtual.
Attachment #8984076 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 25•7 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #24)
> Comment on attachment 8984076 [details] [diff] [review]
> 0001-Bug-1445659-Make-it-possible-to-store-RefPtr-T-in-Au.patch
>
> Review of attachment 8984076 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Thank you for writing the tests!
>
> ::: mfbt/tests/gtest/TestLinkedList.cpp
> @@ +23,5 @@
> > + {
> > + EXPECT_TRUE(!*mResult);
> > + }
> > +
> > + virtual ~PtrClass() {
>
> Does this really need to be virtual? Or is this to work around some
> compiler complaint? I think you could just make `PtrClass` `final`, and
> then the destructor doesn't need to be virtual.
>
> Oh, bother, you inherit below. OK, never mind!
>
> @@ +65,5 @@
> > +{
> > +public:
> > + int mCount;
> > + void AddRef() { mCount++; }
> > + void Release() { mCount--; }
>
> It is weird to see a Release() method that doesn't destroy the class, but OK!
>
> @@ +71,5 @@
> > + CountedClass()
> > + : mCount(0)
> > + {
> > + }
> > + virtual ~CountedClass() { EXPECT_TRUE(mCount == 0); }
>
> Same reasoning applies here, with respect to declaring the class `final` and
> the destructor non-virtual.
Right, this could be final. I'll make it so. The reason for the other being virtual is exactly to test that the right destructor gets called in AutoCleanLinkedList (the bug I found was actually due to that).
The fact that the refcounted doesn't destroy itself is because I want to be able to inspect counts.
I borrowed heavily from the cppunittests in the folder above.
Assignee | ||
Comment 26•6 years ago
|
||
r+ carried over, fixed testcase to use final.
Attachment #8984076 -
Attachment is obsolete: true
Attachment #8985152 -
Flags: review+
Comment 27•6 years ago
|
||
Comment on attachment 8983526 [details] [diff] [review]
0002-Bug-1445659-Create-basic-Browsing-Context-in-Content.patch
Review of attachment 8983526 [details] [diff] [review]:
-----------------------------------------------------------------
::: docshell/base/BrowsingContext.cpp
@@ +30,5 @@
> + }
> +
> + if (!sBrowsingContexts) {
> + sBrowsingContexts =
> + new nsDataHashtable<nsUint64HashKey, BrowsingContext*>();
Shouldn't this be cleared on shutdown too? Seems like we're leaking the hashtable otherwise.
@@ +115,5 @@
> +void
> +BrowsingContext::SetName(const nsAString& aName)
> +{
> + mName = aName;
> +}
Might make sense to inline these simple functions.
::: docshell/base/BrowsingContext.h
@@ +17,5 @@
> +class nsIDocShell;
> +
> +namespace mozilla {
> +
> +class BrowsingContext
This could use a comment describing what the class represents and how it's used. Probably also explaining some of the syncing that happens across processes.
@@ +33,5 @@
> + BrowsingContext(uint64_t aBrowsingContextId, const nsAString& aName);
> +
> + void Attach(BrowsingContext* aParent);
> + void Detach();
> +
Please document what this actually does.
::: docshell/base/nsDocShell.cpp
@@ +3715,5 @@
> nsresult rv = RemoveChildLoader(childAsDocLoader);
> NS_ENSURE_SUCCESS(rv, rv);
>
> + nsCOMPtr<nsIDocShell> childAsDocShell(do_QueryInterface(aChild));
> + if (!childAsDocShell) {
This is backwards! (Looks like we never detach here in practice?)
::: dom/ipc/ContentChild.cpp
@@ +3769,5 @@
> }
> }
>
> +mozilla::ipc::IPCResult
> +ContentChild::RecvAttachBrowsingContext(
Do we really need this on ContentParent *and* ContentChild? The patch seems to only send from child to parent?
::: dom/ipc/ContentParent.cpp
@@ +5727,5 @@
> +ContentParent::RecvAttachBrowsingContext(
> + const BrowsingContextId& aParentId,
> + const BrowsingContextId& aChildId,
> + const nsString& aName)
> +{
Do we need to do some checking on the IDs here? I wonder what would happen if a compromised child process ends up spoofing aParentId and/or aChildId.
Attachment #8983526 -
Flags: review?(peterv) → review-
Assignee | ||
Comment 28•6 years ago
|
||
Addressed review comments, and perhaps most importantly took a stab at some verification of process/context ids. Also moved BrowsingContext from mozilla to mozilla::dom as per Nika's request. Also added logging, to be able to see what's happening.
The changes grew a bit out of scope so I'll attach an interdiff.
Attachment #8983526 -
Attachment is obsolete: true
Attachment #8986453 -
Flags: review?(peterv)
Assignee | ||
Comment 29•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Attachment #8986454 -
Attachment mime type: text/x-patch → text/plain
Assignee | ||
Comment 30•6 years ago
|
||
Rebased and fixed some namespace errors.
Attachment #8986453 -
Attachment is obsolete: true
Attachment #8986453 -
Flags: review?(peterv)
Attachment #8986822 -
Flags: review?(peterv)
Assignee | ||
Comment 31•6 years ago
|
||
... and updated the interdiff.
Assignee | ||
Updated•6 years ago
|
Attachment #8986454 -
Attachment is obsolete: true
Comment 32•6 years ago
|
||
Comment on attachment 8986822 [details] [diff] [review]
0002-Bug-1445659-Create-basic-Browsing-Context-in-Content.patch
Review of attachment 8986822 [details] [diff] [review]:
-----------------------------------------------------------------
::: docshell/base/BrowsingContext.h
@@ +85,5 @@
> + ~BrowsingContext();
> +
> + const uint64_t mBrowsingContextId;
> +
> + // Indicates which process that owns the docshell. Only valid in the
s/that//
::: docshell/base/nsDocShell.cpp
@@ +477,5 @@
>
> rv = nsDocLoader::AddDocLoaderAsChildOfRoot(this);
> NS_ENSURE_SUCCESS(rv, rv);
>
> + mBrowsingContext = new mozilla::dom::BrowsingContext(this);
This file has |using namespace mozilla::dom;| so no need for mozilla::dom::, here and further down.
@@ +5353,5 @@
> mSessionHistory->EvictLocalContentViewers();
> mSessionHistory = nullptr;
> }
>
> + mBrowsingContext->Detach();
So I guess this will Detach twice for "child" BrowsingContexts? (Destroy also calls RemoveChild)
::: dom/ipc/ContentParent.cpp
@@ +5757,5 @@
> + // we want? Maybe we want to crash the child currently calling
> + // SendAttachBrowsingContext and/or the child that originally
> + // called SendAttachBrowsingContext or possibly all children that
> + // has a BrowsingContext connected to the child that currently
> + // called SendAttachBrowsingContext?
Can you file a bug on this? I'm worried we'll ship without making a real decision here.
::: dom/ipc/PContent.ipdl
@@ +1172,5 @@
> + * BrowsingContext effectively does nothing. Note that it is not
> + * an error to call DetachBrowsingContext on a BrowsingContext
> + * belonging to an already detached subtree.
> + */
> + async DetachBrowsingContext(BrowsingContextId aContextId);
Maybe move these to just under AddPerformanceMetrics, that way there's only 1 |parent:| block.
Attachment #8986822 -
Flags: review?(peterv) → review+
Comment 33•6 years ago
|
||
Comment on attachment 8986822 [details] [diff] [review]
0002-Bug-1445659-Create-basic-Browsing-Context-in-Content.patch
Review of attachment 8986822 [details] [diff] [review]:
-----------------------------------------------------------------
::: docshell/base/BrowsingContext.cpp
@@ +80,5 @@
> +{
> + if (isInList()) {
> + MOZ_LOG(GetLog(),
> + LogLevel::Debug,
> + ("%s: Connecting already existing 0x%08lx to 0x%08lx\n",
All these |0x%08lx| fail to compile for me on OS X, I replaced them locally with |0x%08" PRIx64 "|.
::: docshell/base/nsIDocShell.idl
@@ +17,5 @@
> #include "nsIURI.h"
> class nsPresContext;
> class nsIPresShell;
> namespace mozilla {
> +class BrowsingContext;
This now needs to be in the dom namespace block below.
Assignee | ||
Comment 34•6 years ago
|
||
Fixed spelling, namespace, format string, added bug for illegal ops handling (it's Bug 1471598), and moved declarations in PContent.
Carrying over r+
Attachment #8986822 -
Attachment is obsolete: true
Attachment #8986824 -
Attachment is obsolete: true
Attachment #8988185 -
Flags: review+
Assignee | ||
Comment 35•6 years ago
|
||
Attachment #8988189 -
Flags: review?(peterv)
Assignee | ||
Comment 36•6 years ago
|
||
Some test code that adds getAbstractBrowsingContext and getAllAbstractBrowsingContexts to Window. They both return objects describing the BC, useful to JSON.stringify. I usually call them from the console, which makes it possible to inspect the child process (given that you have a document loaded), but loading for exacmple about:config and calling either of the will give the BC's of the parent.
Assignee | ||
Comment 37•6 years ago
|
||
Have BrowsingContext.h be exported to mozilla/dom/, after discussion with peterv.
Carrying over r+
Attachment #8988185 -
Attachment is obsolete: true
Attachment #8988218 -
Flags: review+
Assignee | ||
Comment 38•6 years ago
|
||
And the final fixup for today (I promise): added a null check in ~BrowsingContext:
BrowsingContext::~BrowsingContext()
{
MOZ_DIAGNOSTIC_ASSERT(!isInList());
- sBrowsingContexts->Remove(mBrowsingContextId);
+
+ if (sBrowsingContexts) {
+ sBrowsingContexts->Remove(mBrowsingContextId);
+ }
}
Needed if a shutdown cycle collection happens after sBrowsingContexts is cleared.
Carrying over r+, hope that's ok Peter.
Attachment #8988218 -
Attachment is obsolete: true
Attachment #8988228 -
Flags: review+
Comment 39•6 years ago
|
||
Comment on attachment 8988189 [details] [diff] [review]
0003-Bug-1445659-Make-BrowsingContext-interact-with-bfcac.patch
Review of attachment 8988189 [details] [diff] [review]:
-----------------------------------------------------------------
::: docshell/base/BrowsingContext.cpp
@@ +164,5 @@
> + false /* aMoveToBFCache */);
> +}
> +
> +void
> +BrowsingContext::Cache()
I think this should be named CacheChildren or something
@@ +192,5 @@
> + true /* aMoveToBFCache */);
> +}
> +
> +bool
> +BrowsingContext::IsCached()
This needs to be named differently (RemoveFromCache?). It doesn't just return whether it's cached, it removes it.
::: dom/ipc/PContent.ipdl
@@ +1165,5 @@
> * an error to call DetachBrowsingContext on a BrowsingContext
> * belonging to an already detached subtree.
> */
> + async DetachBrowsingContext(BrowsingContextId aContextId,
> + bool aMoveToBFCache);
Need to document aMoveToBFCache.
Attachment #8988189 -
Flags: review?(peterv) → review+
Assignee | ||
Comment 40•6 years ago
|
||
Attachment #8988228 -
Attachment is obsolete: true
Attachment #8988465 -
Flags: review+
Assignee | ||
Comment 41•6 years ago
|
||
Fixed review issues + a crasher due to accessing sCachedBrowsingContexts in BrowsingContext::Detach() after shutdown.
Carrying over r+
Attachment #8988189 -
Attachment is obsolete: true
Attachment #8988467 -
Flags: review+
Assignee | ||
Comment 42•6 years ago
|
||
Added cleanup of BrowsingContexts when child processes go away.
I'm note sure if I'm happy with the solution, with a lot of root contexts iterating over that list might take too much time?
It would be fairly easy to instead idle-dispatch a runnable that does the work incrementally. Also, since cleanup acts on root contexts it doesn't handle the case where non-root contexts, that should be cleaned, appear in several child processes. For that I filed Bug 1472108.
Attachment #8988690 -
Flags: review?(peterv)
Assignee | ||
Comment 43•6 years ago
|
||
Patch number 4 in this patch series iterates over _all_ BrowsingContexts in the ContentParent when a ContentChild goes away, and this might be sub-optimal. This patch initiates an incremental cleanup sequence using idle callbacks.
If we decide to go with this instead it needs a bit more tweaking to determine the best timeout, etc. I'm not saying we should, but I thought I'd attach it anyway.
Assignee | ||
Comment 44•6 years ago
|
||
Try is looking greenish: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ea3c42607f4d959b6f6437e40d5efd423811dcf&group_state=expanded&selectedJob=185577345
There are some intermittents that insist on not going away after retries.
Assignee | ||
Updated•6 years ago
|
Attachment #8988690 -
Flags: review?(peterv) → review?(nika)
Comment 45•6 years ago
|
||
Comment on attachment 8988690 [details] [diff] [review]
0004-Bug-1445659-Remove-dangling-BrowsingContexts-left-fr.patch
Review of attachment 8988690 [details] [diff] [review]:
-----------------------------------------------------------------
::: docshell/base/BrowsingContext.cpp
@@ +67,5 @@
> +// cleaned. [Bug 1472108]
> +/* static */ void
> +BrowsingContext::CleanupContexts(uint64_t aProcessId)
> +{
> + if (sRootBrowsingContexts) {
I think in the future we will want to keep these BrowsingContexts around even after the browser has crashed for use in sessionrestore, but I think this is good for now :-)
Attachment #8988690 -
Flags: review?(nika) → review+
Assignee | ||
Comment 46•6 years ago
|
||
Fixed rebase conflicts, carrying over r+
Attachment #8988465 -
Attachment is obsolete: true
Attachment #8995269 -
Flags: review+
Assignee | ||
Comment 47•6 years ago
|
||
Try looks like this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c482cbb181b73fd4fbfcc47db1fa7214471aa265&group_state=expanded
There are some permafailing intermittents that I haven't diagnosed. Not sure what to make of it.
Assignee | ||
Comment 48•6 years ago
|
||
So I ran a try run for the permafailing tests of the parent commit of the patch series, and it looks like those problems are there too[1]. So I guess that this is green enough on try.
[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f09b7baa7f4cd75f78c9b97e4f40cd898e99043&group_state=expanded
Keywords: checkin-needed
Comment 49•6 years ago
|
||
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c5f1bec2005
Make it possible to store RefPtr<T> in AutoCleanLinkedList. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/c70a61e3f34a
Create basic Browsing Context in Content Parent and Child. r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/10b4f3fa453f
Make BrowsingContext interact with bfcache. r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e77586d8615
Remove dangling BrowsingContexts left from closing process. r=Nika
Keywords: checkin-needed
Comment 50•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9c5f1bec2005
https://hg.mozilla.org/mozilla-central/rev/c70a61e3f34a
https://hg.mozilla.org/mozilla-central/rev/10b4f3fa453f
https://hg.mozilla.org/mozilla-central/rev/5e77586d8615
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Updated•6 years ago
|
Flags: needinfo?(peterv)
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•