Stop doing sync IPC for PContent::Msg_CreateWindow

RESOLVED FIXED in Firefox 56

Status

()

RESOLVED FIXED
2 years ago
5 days ago

People

(Reporter: Ehsan, Assigned: Nika)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [qf:p1])

Attachments

(4 attachments, 14 obsolete attachments)

8.11 KB, patch
Details | Diff | Splinter Review
1.65 KB, patch
Details | Diff | Splinter Review
6.17 KB, patch
smaug
: review+
Details | Diff | Splinter Review
23.85 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

2 years ago
This sync IP seems to be very bad: <https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-02-22&keys=PContent%253A%253AMsg_CreateWindow!PCookieService%253A%253AMsg_GetCookieString!PContent%253A%253AMsg_RpcMessage!PAPZCTreeManager%253A%253AMsg_ReceiveMouseInputEvent&max_channel_version=nightly%252F54&measure=IPC_SYNC_LATENCY_MS&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-02-02&table=0&trim=1&use_submission_date=0>

Start	End	IPC_SYNC_LATENCY_MS Count
0	1	0 (0%)
1	3	1.83k (0.05%)
3	8	12.22k (0.31%)
8	20	1.17M (29.51%)
20	50	1.47M (36.99%)
50	126	491.33k (12.37%)
126	317	564.32k (14.21%)
317	796	219.88k (5.54%)
796	2k	35.43k (0.89%)
2k	Infinity	5.8k (0.15%)

This affects both normal navigations and window.open().

I wrote a patch to fix this but the try server is extremely unhappy with it.  I need to find some time to fix it up and submit it for review.
Can you post what you've got so far to this bug, ehsan? I might be able to help push it over the line (I've wrestled with window opening before).
Flags: needinfo?(ehsan)
(Reporter)

Comment 2

2 years ago
I'll post my patch so far in a moment but it's not very useful since I'm not sure how close it is yet.  The patch works well for basic browsing but breaks all of our test suites and I haven't yet had a chance to debug all of the broken tests.  I'm still planning on doing that.
Flags: needinfo?(ehsan)
(Reporter)

Comment 3

2 years ago
Posted patch WIP (part 1) (obsolete) — Splinter Review
(Reporter)

Comment 4

2 years ago
Posted patch WIP (part 2) (obsolete) — Splinter Review
(Reporter)

Updated

2 years ago
Blocks: 1347035
Whiteboard: [qf:p1]
(Reporter)

Updated

2 years ago
Blocks: 1350633
(Reporter)

Updated

2 years ago
Depends on: 1356922
(Reporter)

Comment 5

2 years ago
Some status update on this, I was stuck trying out various strategies for making the sNextTabParent setup work with these patches to no success for quite a while so far.  Over the weekend I ended up giving up on that approach and decided to just remove sNextTabParent completely and just not use a global variable like this in the first place.  That fix is now up for review in bug 1356922 so that I can land it sooner than this is ready.  With that, the test failure I was debugging for the past few weeks is now working, and it's time for another round of try pushes of this patch!
(Reporter)

Updated

2 years ago
Depends on: 1362760
(Reporter)

Comment 6

2 years ago
I finally found the root cause of some of the test failures I have been chasing for weeks here!  Bug 1362760 suggests a possible fix.  There seems to be a few more test failures, but I'll probably wait to figure out how to deal with bug 1362760 first.
(Assignee)

Updated

2 years ago
Blocks: 1365032
(Reporter)

Comment 7

2 years ago
Posting the latest version of the patches I have here because mystor needs them.
(Reporter)

Updated

2 years ago
Attachment #8846848 - Attachment is obsolete: true
(Reporter)

Updated

2 years ago
Attachment #8846849 - Attachment is obsolete: true
(Assignee)

Comment 9

2 years ago
This will probably land after the noopener in new process work I'm doing, so I'm swapping the dependency order.
No longer blocks: 1365032
Depends on: 1365032
(Assignee)

Comment 10

2 years ago
Do you have a tree with the changes applied posted somewhere which I could more easily test with? I tried replicating the changes you were making on a local tree (with a simple patch), and while it correctly opens a window currently on my computer it breaks rendering in the new page. I don't see the substantial difference between what I wrote compared to what you did so I'm curious as to what might be going on.
Flags: needinfo?(ehsan)
(Reporter)

Comment 11

2 years ago
I gave my tree to Michael on IRC.
Flags: needinfo?(ehsan)
(Assignee)

Comment 12

2 years ago
I'm going to be taking this bug from Ehsan for now.
Assignee: ehsan → michael
(Reporter)

Comment 13

2 years ago
Thank you!

Do you still need the fix to bug 1362760?
(Assignee)

Comment 14

2 years ago
This patch does the work to remove the IPC and instead replace it with a nested
event loop. It tries to do the minimal amount of work to accomplish this, and
there is still a sync IPC for the <iframe mozbrowser> case right now.

There was a problem where we would set the docshell to be active too early after
this patch, as the document wouldn't have received the response from the open
call before it is made active. In order to fix this code was added to delay
setting the docshell to be active until after the nested event loop is done
spinning.

Unlike the original patch which ehsan wrote, this patch does not expose an async
API from nsWindowProvider. We can look into adding an API like that later, but
for now keeping the nested event loop inside of ContentChild::ProvideWindow
seems like the easiest option.

MozReview-Commit-ID: J3GaYaMx9P1

------------

I am also asking for review from billm for the change to sync-messages, as I need an IPC peer to review that change.
Attachment #8877234 - Flags: review?(wmccloskey)
Attachment #8877234 - Flags: review?(ehsan)
(Assignee)

Updated

2 years ago
Attachment #8868744 - Attachment is obsolete: true
Attachment #8868745 - Attachment is obsolete: true
(Assignee)

Comment 15

2 years ago
Only asking for billm to review the change to sync-messages as I need an IPC peer to review that change.

MozReview-Commit-ID: JRPgQt2tbNz
Attachment #8877236 - Flags: review?(wmccloskey)
Attachment #8877236 - Flags: review?(ehsan)
(Assignee)

Comment 16

2 years ago
(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog) from comment #13)
> Thank you!
> 
> Do you still need the fix to bug 1362760?

I do not need that fix anymore with this patch :)
(Assignee)

Comment 17

2 years ago
I accidentally included a printf in the last version :-S.

Again, r?billm only for sync-messages changes

MozReview-Commit-ID: E9horwljLAU
Attachment #8877242 - Flags: review?(wmccloskey)
Attachment #8877242 - Flags: review?(ehsan)
(Assignee)

Updated

2 years ago
Attachment #8877234 - Attachment is obsolete: true
Attachment #8877234 - Flags: review?(wmccloskey)
Attachment #8877234 - Flags: review?(ehsan)
Comment on attachment 8877242 [details] [diff] [review]
Part 1: Remove the Msg_CreateWindow sync IPC

Review of attachment 8877242 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/ipc/PContent.ipdl
@@ +992,5 @@
>       */
>      sync GetGraphicsDeviceInitData()
>          returns (ContentDeviceData aData);
>  
> +    async CreateWindow(nullable PBrowser aThisTab,

This message seems like a perfect place to use the new promise stuff that Kan-Ru added recently. You can have CreateWindow return a promise. Then you won't need to keep the hashtable in ContentChild. The IPC code will do that for you.

::: dom/ipc/TabChild.cpp
@@ +2405,5 @@
>  
> +  // If we're currently waiting for window opening to complete, we need to hold
> +  // off on setting the docshell active. We queue up the values we're receiving
> +  // in the mWindowOpenDocShellActiveStatus.
> +  if (mPendingDocShellBlockers > 0) {

How about moving everything after this block to a separate method? Then you can call that method in RemovePendingDocShellBlocker and not have to worry about setting the epoch properly. You'll have to s/aLayerObserverEpoch/mLayerObserverEpoch/ in a few places.
Attachment #8877242 - Flags: review?(wmccloskey) → review+
Attachment #8877236 - Flags: review?(wmccloskey) → review+
(Assignee)

Comment 19

2 years ago
MozReview-Commit-ID: KAaeu5ETc0x
Attachment #8878124 - Flags: review?(bugs)
(Assignee)

Updated

2 years ago
Attachment #8877236 - Attachment is obsolete: true
Attachment #8877242 - Attachment is obsolete: true
Attachment #8877236 - Flags: review?(ehsan)
Attachment #8877242 - Flags: review?(ehsan)
(Assignee)

Comment 20

2 years ago
This is needed for part 3, when I add an ipdl struct which contains these types. IPDL structs require all members to have a const operator== defined on them, so I added it.

MozReview-Commit-ID: 6BayMmqNlyx
Attachment #8878126 - Flags: review?(bugs)
(Assignee)

Comment 21

2 years ago
MozReview-Commit-ID: IWayDUWuRrf
Attachment #8878127 - Flags: review?(bugs)
Comment on attachment 8878124 [details] [diff] [review]
Part 1: Add the ability to temporarially delay remote docshells from becoming active

The comment about hang monitor is weird and probably wrong, but you just moved it.
Attachment #8878124 - Flags: review?(bugs) → review+
Comment on attachment 8878126 [details] [diff] [review]
Part 2: Add const operator== to TextureFactoryIdentifier and CompositorOptions

Some gfx dev should technically review this.
{ goes to its own line after method definition.
Attachment #8878126 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #22)
> Comment on attachment 8878124 [details] [diff] [review]
> Part 1: Add the ability to temporarially delay remote docshells from
> becoming active
> 
> The comment about hang monitor is weird and probably wrong, but you just
> moved it.

Why is the comment wrong?
Because the request isn't coming from hang monitor channel. Sure, we seem to reuse the same method for ipc stuff and for non-ipc. Misleading comment, I'd say.
(In reply to Olli Pettay [:smaug] from comment #25)
> Because the request isn't coming from hang monitor channel. Sure, we seem to
> reuse the same method for ipc stuff and for non-ipc. Misleading comment, I'd
> say.

I guess it would be fine to say "Since requests to change the active docshell come in from both...". Is that better?
Comment on attachment 8878127 [details] [diff] [review]
Part 3: Remove the window creation sync IPC calls, r=billm

Review of attachment 8878127 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/ipc/ContentChild.cpp
@@ +902,5 @@
> +  // Await the promise being resolved. When the promise is resolved, we'll set
> +  // the `ready` local variable, which will cause us to exit our nested event
> +  // loop.
> +  bool ready = false;
> +  windowCreated->Then(AbstractThread::MainThread(), __func__,

Please don't use AbstractThread here. Now you can use any nsISerialEventTarget. If bug 1372733 lands before you, then you can use SystemGroup::EventTargetFor(Other). Otherwise just use GetMainThreadSerialEventTarget().
Comment on attachment 8878127 [details] [diff] [review]
Part 3: Remove the window creation sync IPC calls, r=billm

>+  // Await the promise being resolved. When the promise is resolved, we'll set
>+  // the `ready` local variable, which will cause us to exit our nested event
>+  // loop.
>+  bool ready = false;
>+  windowCreated->Then(AbstractThread::MainThread(), __func__,
>+                      [&] (const CreatedWindowInfo& info) {
>+                        rv = info.rv();
>+                        *aWindowIsNew = info.windowOpened();
>+                        frameScripts = info.frameScripts();
>+                        urlToLoad = info.urlToLoad();
>+                        textureFactoryIdentifier = info.textureFactoryIdentifier();
>+                        layersId = info.layersId();
>+                        compositorOptions = info.compositorOptions();
>+                        maxTouchPoints = info.maxTouchPoints();
>+                        dimensionInfo = info.dimensions();
>+                        ready = true;
>+                      },
>+                      [&] (const CreateWindowPromise::RejectValueType aReason) {
>+                        NS_WARNING("windowCreated promise rejected");
>+                        rv = NS_ERROR_NOT_AVAILABLE;
>+                        ready = true;
>+                      });
(hurts my eyes and soul)


Looks surprisingly simple.
Attachment #8878127 - Flags: review?(bugs) → review+
(Assignee)

Comment 29

2 years ago
Comment on attachment 8878126 [details] [diff] [review]
Part 2: Add const operator== to TextureFactoryIdentifier and CompositorOptions

Asking jeff for a gfx r+
Attachment #8878126 - Flags: review?(jmuizelaar)
Attachment #8878126 - Flags: review?(jmuizelaar) → review+
(Assignee)

Updated

2 years ago
Attachment #8878124 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Attachment #8878126 - Attachment is obsolete: true
Attachment #8878127 - Attachment is obsolete: true
(Assignee)

Comment 31

2 years ago
MozReview-Commit-ID: 6BayMmqNlyx
(Assignee)

Comment 32

2 years ago
MozReview-Commit-ID: 6BayMmqNlyx
(Assignee)

Updated

2 years ago
Attachment #8878626 - Attachment is obsolete: true
(Assignee)

Comment 33

2 years ago
MozReview-Commit-ID: IWayDUWuRrf

Comment 34

2 years ago
Pushed by michael@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5573f2f7d796
Part 1: Add the ability to temporarially delay remote docshells from becoming active, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/0bade2cd1908
Part 2: Add const operator== to TextureFactoryIdentifier and CompositorOptions, r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/05a5f9d8249f
Part 3: Remove the window creation sync IPC calls, r=billm, r=smaug
Backed out bug 1343728 and bug 1350633 for failing various tests, e.g. mochitest test_animations_reverse.html:

https://hg.mozilla.org/integration/mozilla-inbound/rev/105d12cd1d6d72f4f3a52584c909bff34e8e43fa
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e927a18b19c9b48bcd8e5b83895b078bbc051ba
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e623a352cbb73651b3a18549f3096247cb310ad
https://hg.mozilla.org/integration/mozilla-inbound/rev/42ce4647768307b20114899b8c3de8c622791342

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=956306ea34f36514350ce6f03c114eedbabe934f&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure example: https://treeherder.mozilla.org/logviewer.html#?job_id=107791530&repo=mozilla-inbound
[task 2017-06-16T19:32:30.922114Z] 19:32:30     INFO - TEST-START | layout/style/test/test_animations_reverse.html
[task 2017-06-16T19:32:31.502503Z] 19:32:31     INFO - TEST-INFO | started process screentopng
[task 2017-06-16T19:32:33.217196Z] 19:32:33     INFO - TEST-INFO | screentopng: exit 0
[task 2017-06-16T19:32:33.220930Z] 19:32:33     INFO - TEST-UNEXPECTED-FAIL | layout/style/test/test_animations_reverse.html | OMTA should work 
[task 2017-06-16T19:32:33.222139Z] 19:32:33     INFO -     runOMTATest/<@layout/style/test/animation_utils.js:217:9
[task 2017-06-16T19:32:33.223374Z] 19:32:33     INFO -     promise callback*runOMTATest@layout/style/test/animation_utils.js:209:3
[task 2017-06-16T19:32:33.224574Z] 19:32:33     INFO -     @layout/style/test/file_animations_reverse.html:36:1
Flags: needinfo?(michael)
(Assignee)

Comment 36

2 years ago
This ended up being really really red on inbound, while it seemed fine when I did various try pushes with it - looking into what might have changed. I think this might be related to the changes to using promises rather than the previous version which did not.
(Assignee)

Comment 37

2 years ago
I somehow broke this between the version in comment 14, and the version I asked for review on. I'm going to look into the semantic differences between these versions on monday.
(Assignee)

Comment 38

2 years ago
I took a look into this failure. It seems like this is an intermittent failure, and is caused by the change to the promise-based version from the non-promise based one. From what I can tell, the promise based one messes us up by breaking some of the ordering guarantees which I was depending on. Namely, once RecvCreateWindow exits, I was expecting all messages being sent from the parent to arrive in the content after ProvideWindow has returned. With the old system this was the case, however what happens now is we send the Reply_CreateWindow message at the right time, and the subsequent messages come after it, however that message just fulfils the promise, and the actual event loop is not exited until a new event reaches the front of the event queue, which can totally screw up ordering, as other messages can also have arrived, and end up being processed within the nested event loop when they should not be.

I think the easiest way to fix this would be to either 

A) have some mechanism for polling the promise to see if it has been resolved ourselves on every pass of the event loop, which would allow us to pull the values out and continue synchronously, or
B) go back to the original design which manually sent the response message and managed identifiers, or
C) Change the logic somehow to make the IO thread which receives the message resolve the promise there, ensuring that the runnables for the resolved promise appear on the main thread before any further messages are received, and then change the `Then` runnable to dispatch to an unlabeled event target to avoid any ordering issues.

Billm/smaug, what would you rather that I do? I think my favorite would be B, as it doesn't require too massive of a change (just exposing a Poll method on MozResult, I don't think we have something like that right now.),

C) would also work well, as it makes the promise return values DTRT, which seems like a good thing to me, and removes some of the overhead I would think, however I have no idea how to do it, and it would probably delay this patch a bit. 

A) would kinda suck to do, but require the least changes to promises and IPDL, which might be desirable. In addition, it decreases the number of lambdas in the patch substantially, which I know smaug is a fan of.
Flags: needinfo?(wmccloskey)
Flags: needinfo?(michael)
Flags: needinfo?(bugs)
(Assignee)

Comment 39

2 years ago
(In reply to Michael Layzell [:mystor] from comment #38)
> A) have some mechanism for polling the promise to see if it has been
> resolved ourselves on every pass of the event loop, which would allow us to
> pull the values out and continue synchronously, or
> B) go back to the original design which manually sent the response message
> and managed identifiers, or
> C) Change the logic somehow to make the IO thread which receives the message
> resolve the promise there, ensuring that the runnables for the resolved
> promise appear on the main thread before any further messages are received,
> and then change the `Then` runnable to dispatch to an unlabeled event target
> to avoid any ordering issues.
> 
> Billm/smaug, what would you rather that I do? I think my favorite would be
> B, as it doesn't require too massive of a change (just exposing a Poll
> method on MozResult, I don't think we have something like that right now.),
> 
> C) would also work well, as it makes the promise return values DTRT, which
> seems like a good thing to me, and removes some of the overhead I would
> think, however I have no idea how to do it, and it would probably delay this
> patch a bit. 
> 
> A) would kinda suck to do, but require the least changes to promises and
> IPDL, which might be desirable. In addition, it decreases the number of
> lambdas in the patch substantially, which I know smaug is a fan of.

In my comments above about the options, I swapped A and B in my head. Oops.
I filed bug 1374368 to find a long-term solution to the ordering thing. For now I think it's fine to land with the hashtable solution.
Flags: needinfo?(wmccloskey)
Flags: needinfo?(bugs)
(Assignee)

Comment 41

2 years ago
This is a change to MozPromise that I need in order to keep the ordering while continuing to use MozPromise. The other option which I have if I can't make this change is to re-rewrite this patch to use explicit message passing back and forth again, with a HashTable in the ContentChild object.

I opted to do it this way to avoid the need to make as major of a change, but I can if we decide we definitely don't want a method like this on MozPromise.

MozReview-Commit-ID: 5UTdYywYId6
Attachment #8879300 - Flags: review?(jwwang)
(Assignee)

Comment 42

2 years ago
This depends on Part 4, and changes the code in part 3 to work correctly.

MozReview-Commit-ID: 9PuPNyt2GNV
Attachment #8879301 - Flags: review?(bugs)
So you didn't take the approach we discussed on IRC.
I think Poll is quite ugly, but I guess fine until someone figures out nicer API to handle this all.
Attachment #8879301 - Flags: review?(bugs) → review+
Comment on attachment 8879300 [details] [diff] [review]
Part 4: Add a Poll method to MozPromise to check if a promise is resolved

Review of attachment 8879300 [details] [diff] [review]:
-----------------------------------------------------------------

I think this change reintroduces the race condition which promise aims to eliminate. Checking the pending status of a promise increases the chance of cross-thread reads and is basically racy for it could be changed by another thread any time soon. Maybe bobby has some more ideas about this new API.
Attachment #8879300 - Flags: review?(jwwang) → feedback?(bobbyholley)
(Assignee)

Comment 46

2 years ago
Comment on attachment 8879300 [details] [diff] [review]
Part 4: Add a Poll method to MozPromise to check if a promise is resolved

I'm going to not use this new API. I got confused after olli and my conversation on IRC and I thought this was the API he wanted me to use, but the other suggestions seem nicer.
Attachment #8879300 - Attachment is obsolete: true
Attachment #8879300 - Flags: feedback?(bobbyholley)
(Assignee)

Updated

2 years ago
Attachment #8879301 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Attachment #8878629 - Attachment is obsolete: true
(Assignee)

Comment 48

2 years ago
MozReview-Commit-ID: IWayDUWuRrf
So does something guarantee no scripts run during stable state handling?
Also, note, stable state scheduling in Gecko is wrong. It will be microtask checkpoint, not end of task hopefully soon.
(Assignee)

Comment 50

2 years ago
(In reply to Olli Pettay [:smaug] from comment #49)
> So does something guarantee no scripts run during stable state handling?
> Also, note, stable state scheduling in Gecko is wrong. It will be microtask
> checkpoint, not end of task hopefully soon.

There's nothing guaranteeing that no scripts run in the dispatcher. There is a comment on `GetStableStateEventDispatcher` which mentions that no event dispatched to it is allowed to run scripts.

In our particular case, it is guaranteed because the only thing which is dispatched is the Then callback from the MozPromise, which only performs variable assignment, and cannot run script.
Comment on attachment 8879733 [details] [diff] [review]
Part 3: Add StableStateEventTarget, an event target which runs dispatched runnables at the next stable state

I guess we can try this, and for the particular use case here it shouldn't matter that the timing of stable state will change.
Attachment #8879733 - Flags: review?(bugs) → review+

Comment 52

2 years ago
Pushed by michael@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5630ef01b17
Part 1: Add the ability to temporarially delay remote docshells from becoming active, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/831e95b29f66
Part 2: Add const operator== to TextureFactoryIdentifier and CompositorOptions, r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/0169e09f6030
Part 3: Add StableStateEventTarget, an event target which runs dispatched runnables at the next stable state, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/2029cb6bce86
Part 4: Remove the window creation sync IPC calls, r=billm, r=smaug
I had to back this out because it or something else from the push caused bug 1375293, which was failing more frequently than we could handle.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/14f84d3b3233a87af51f5ad8302ee24cc2371a01
Flags: needinfo?(michael)

Comment 54

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d5630ef01b17
https://hg.mozilla.org/mozilla-central/rev/831e95b29f66
https://hg.mozilla.org/mozilla-central/rev/0169e09f6030
https://hg.mozilla.org/mozilla-central/rev/2029cb6bce86
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(Assignee)

Comment 55

2 years ago
Was this bug fixed or backed out? I'm slightly confused.
Flags: needinfo?(michael) → needinfo?(wkocher)
The technical term for its state is "it was screwed up."

Land on m-i, backed out on m-i, then the cset above its landing, way below its backout, was merged to m-c. Looked at one way, that means it is now fixed, and will be fixed until the backout is merged around. Looked at another way, I'll clean up the mess by grafting the backout to m-c and merging it to autoland, and it'll be backed out for realz.
Status: RESOLVED → REOPENED
status-firefox56: fixed → ---
Flags: needinfo?(wkocher)
Resolution: FIXED → ---
Target Milestone: mozilla56 → ---

Comment 57

2 years ago
Backout by philringnalda@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/dc33e00dad90
Backed out 4 changesets for failures in test_group_mouseevents.html a=backout
(Assignee)

Updated

2 years ago
Depends on: 1375940

Comment 58

2 years ago
Pushed by michael@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9dfba832730
Part 1: Add the ability to temporarially delay remote docshells from becoming active, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3ac91f3aeb9
Part 2: Add const operator== to TextureFactoryIdentifier and CompositorOptions, r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/edbd391db6d8
Part 3: Add StableStateEventTarget, an event target which runs dispatched runnables at the next stable state, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/4364536ae549
Part 4: Remove the window creation sync IPC calls, r=billm, r=smaug

Comment 59

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e9dfba832730
https://hg.mozilla.org/mozilla-central/rev/c3ac91f3aeb9
https://hg.mozilla.org/mozilla-central/rev/edbd391db6d8
https://hg.mozilla.org/mozilla-central/rev/4364536ae549
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
backed out because of test failures after a incomplete backout see https://bugzilla.mozilla.org/show_bug.cgi?id=1375940#c19 and #20
Status: RESOLVED → REOPENED
status-firefox56: fixed → ---
Resolution: FIXED → ---

Comment 62

2 years ago
Backout by cbook@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/b277a2163287
Backed out changeset 4364536ae549 
https://hg.mozilla.org/mozilla-central/rev/46f1cad02172
Backed out changeset edbd391db6d8 
https://hg.mozilla.org/mozilla-central/rev/1c9e892dbd0a
Backed out changeset c3ac91f3aeb9

Comment 63

2 years ago
Pushed by michael@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6bf888fae2fe
Part 1: Add the ability to temporarially delay remote docshells from becoming active, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/f075e418633c
Part 2: Add const operator== to TextureFactoryIdentifier and CompositorOptions, r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9444e4f6cc4
Part 3: Add StableStateEventTarget, an event target which runs dispatched runnables at the next stable state, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8d134e64799
Part 4: Remove the window creation sync IPC calls, r=billm, r=smaug
Depends on: 1378374
Depends on: 1378112
Depends on: 1391211
Depends on: 1412173

Updated

a year ago
Depends on: 1416728
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.