Closed
Bug 1343728
Opened 7 years ago
Closed 7 years ago
Stop doing sync IPC for PContent::Msg_CreateWindow
Categories
(Core :: DOM: Core & HTML, enhancement)
Core
DOM: Core & HTML
Tracking
()
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: ehsan.akhgari, Assigned: nika)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
Attachments
(4 files, 14 obsolete files)
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 |
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.
Comment 1•7 years ago
|
||
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•7 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•7 years ago
|
||
Reporter | ||
Comment 4•7 years ago
|
||
Updated•7 years ago
|
Whiteboard: [qf:p1]
Reporter | ||
Comment 5•7 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 | ||
Comment 6•7 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.
Reporter | ||
Comment 7•7 years ago
|
||
Posting the latest version of the patches I have here because mystor needs them.
Reporter | ||
Updated•7 years ago
|
Attachment #8846848 -
Attachment is obsolete: true
Reporter | ||
Comment 8•7 years ago
|
||
Reporter | ||
Updated•7 years ago
|
Attachment #8846849 -
Attachment is obsolete: true
Assignee | ||
Comment 9•7 years ago
|
||
This will probably land after the noopener in new process work I'm doing, so I'm swapping the dependency order.
Assignee | ||
Comment 10•7 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)
Assignee | ||
Comment 12•7 years ago
|
||
I'm going to be taking this bug from Ehsan for now.
Assignee: ehsan → michael
Reporter | ||
Comment 13•7 years ago
|
||
Thank you! Do you still need the fix to bug 1362760?
Assignee | ||
Comment 14•7 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•7 years ago
|
Attachment #8868744 -
Attachment is obsolete: true
Attachment #8868745 -
Attachment is obsolete: true
Assignee | ||
Comment 15•7 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•7 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•7 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•7 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•7 years ago
|
||
MozReview-Commit-ID: KAaeu5ETc0x
Attachment #8878124 -
Flags: review?(bugs)
Assignee | ||
Updated•7 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•7 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•7 years ago
|
||
MozReview-Commit-ID: IWayDUWuRrf
Attachment #8878127 -
Flags: review?(bugs)
Comment 22•7 years ago
|
||
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 23•7 years ago
|
||
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?
Comment 25•7 years ago
|
||
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 28•7 years ago
|
||
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•7 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)
Updated•7 years ago
|
Attachment #8878126 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 30•7 years ago
|
||
MozReview-Commit-ID: KAaeu5ETc0x
Assignee | ||
Updated•7 years ago
|
Attachment #8878124 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8878126 -
Attachment is obsolete: true
Attachment #8878127 -
Attachment is obsolete: true
Assignee | ||
Comment 31•7 years ago
|
||
MozReview-Commit-ID: 6BayMmqNlyx
Assignee | ||
Comment 32•7 years ago
|
||
MozReview-Commit-ID: 6BayMmqNlyx
Assignee | ||
Updated•7 years ago
|
Attachment #8878626 -
Attachment is obsolete: true
Assignee | ||
Comment 33•7 years ago
|
||
MozReview-Commit-ID: IWayDUWuRrf
Comment 34•7 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
Comment 35•7 years ago
|
||
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•7 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•7 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•7 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•7 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.
See Also: → 1374368
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)
Updated•7 years ago
|
Flags: needinfo?(bugs)
Assignee | ||
Comment 41•7 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•7 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)
Comment 43•7 years ago
|
||
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.
Updated•7 years ago
|
Attachment #8879301 -
Flags: review?(bugs) → review+
Comment 44•7 years ago
|
||
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)
See https://bugzilla.mozilla.org/show_bug.cgi?id=1374368#c10 for a different way to do this.
Assignee | ||
Comment 46•7 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•7 years ago
|
Attachment #8879301 -
Attachment is obsolete: true
Assignee | ||
Comment 47•7 years ago
|
||
MozReview-Commit-ID: 8dlo5Z60qsG
Attachment #8879733 -
Flags: review?(bugs)
Assignee | ||
Updated•7 years ago
|
Attachment #8878629 -
Attachment is obsolete: true
Assignee | ||
Comment 48•7 years ago
|
||
MozReview-Commit-ID: IWayDUWuRrf
Comment 49•7 years ago
|
||
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•7 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 51•7 years ago
|
||
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•7 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•7 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
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Assignee | ||
Comment 55•7 years ago
|
||
Was this bug fixed or backed out? I'm slightly confused.
Flags: needinfo?(michael) → needinfo?(wkocher)
Comment 56•7 years ago
|
||
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•7 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
Comment 58•7 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•7 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
Closed: 7 years ago → 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 60•7 years ago
|
||
backed out because of test failures after a incomplete backout see https://bugzilla.mozilla.org/show_bug.cgi?id=1375940#c19 and #20
Comment 61•7 years ago
|
||
Backout push is here: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=cc903e3f61894e60c3b0efaf05e9a446d1d85888
Comment 62•7 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•7 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
Comment 64•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6bf888fae2fe https://hg.mozilla.org/mozilla-central/rev/f075e418633c https://hg.mozilla.org/mozilla-central/rev/e9444e4f6cc4 https://hg.mozilla.org/mozilla-central/rev/c8d134e64799
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
Updated•2 years ago
|
Performance Impact: --- → P1
Whiteboard: [qf:p1]
You need to log in
before you can comment on or make changes to this bug.
Description
•