Update skia to version 59

RESOLVED FIXED in Firefox 55

Status

()

defect
P3
normal
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: RyanVM, Assigned: lsalzman)

Tracking

(Depends on 1 bug, Blocks 1 bug)

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(12 attachments, 1 obsolete attachment)

52 bytes, text/plain
mchang
: review+
Details
51.74 KB, patch
jrmuizel
: review+
ted
: review+
Details | Diff | Splinter Review
32.49 KB, patch
mchang
: review+
Details | Diff | Splinter Review
5.05 KB, patch
mchang
: review+
Details | Diff | Splinter Review
1.87 KB, patch
mchang
: review+
Details | Diff | Splinter Review
2.23 KB, patch
mchang
: review+
Details | Diff | Splinter Review
3.16 KB, patch
mchang
: review+
Details | Diff | Splinter Review
49.24 KB, patch
mchang
: review+
Details | Diff | Splinter Review
1.53 KB, patch
lsalzman
: review+
Details | Diff | Splinter Review
4.13 KB, patch
lsalzman
: review+
Details | Diff | Splinter Review
3.20 KB, application/vnd.mozilla.xul+xml
Details
5.31 KB, patch
lsalzman
: review+
Details | Diff | Splinter Review
Reporter

Description

2 years ago
(In reply to Tom Ritter [:tjr] from bug 1338658 comment #0)
> This is a (semi-)automated bug making you aware that there is an available
> upgrade for an embedded third-party library.
> 
> skia is currently at version 55 in mozilla-central, and the latest version
> of the library released is 58.
> 
> I fetched the latest version of the library from
> https://skia.googlesource.com/skia/+/master/include/core/SkMilestone.h.
> 
> You can leave this bug open, and it will be updated if a newer version of
> the library becomes available. If you close it as WONTFIX, please indicate
> if you do not wish to receive any future bugs upon new releases of the
> library.

Per discussion with lsalzman, we're not ready to update yet, but getting this bug on file to track it when we are.

Comment 1

2 years ago
Skia 58 has been released.
Assignee

Comment 2

2 years ago
Still waiting on Skia upstream to potentially incorporate some fixes/features before I move on this.
Priority: -- → P3
Whiteboard: [gfx-noted]

Comment 3

2 years ago
Skia 59 has been released.
Summary: Update skia to version 58 → Update skia to version 59

Updated

2 years ago
No longer blocks: 1325608
Assignee

Updated

2 years ago
Depends on: 1350262
Assignee

Comment 4

2 years ago
Nothing too controversial here in terms of my new commits. Just a lot of churn updating old patches to work with m59, and then some new fixes to deal with build issues, and some compat things like the use of std::chrono features that we don't support.
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Attachment #8865115 - Flags: review?(mchang)
Assignee

Comment 5

2 years ago
The main change here is the migration from gyp to gn. So all our gyp-related scripts get to go away... that's the good part.

The most effective way to get gn to do what we want here is to use "gn gen" to first generate the build files for the desired platform, then use "gn desc" to list out the manifests thereof for specific targets (like :effects, :gpu, :pdf, etc).

In the process replaced use of deprecated os.system with the subprocess module, which is ordained way to do such things now.
 
I also cleaned up the generation of opt sources by using the specific optimization targets (:sse2, :armv7, etc) instead of just walking the entire directory which was not terribly safe. This makes binning the files into the right sets in the sources dictionary easier to, since we no longer need to do it in post - they go right to the sorted sets where they need to go from the beginning.

Also of note, NEON is now always used throughout the build. The groundwork for this was taken care of when we migrated all non-NEON ARM users to 52 indefinitely and not letting them progress to 53+. So we've been clear to make that change for a while, and Milan has given the go-ahead for it.
Attachment #8865116 - Flags: review?(jmuizelaar)
Comment on attachment 8865116 [details] [diff] [review]
update moz.build for Skia m59

Probably good to get a build peer to look at this.
Attachment #8865116 - Flags: review?(ted)
Assignee

Comment 7

2 years ago
A bunch of fun DrawTargetSkia changes herein...

They no longer support refcounting SkCanvas anymore, and their ordained way of managing it is with std::unique_ptr. BUT, SkSurface already owns the std::unique_ptr to it, so if you get an SkCanvas from the SkSurface, you're supposed to just keep the SkSurface around instead, and just point directly to the SkCanvas. We already keep around the SkSurface, so main hassle here is just getting rid of the sk_sp (shared pointer) junk. The only exception to this being the case of SkCanvas::MakeRasterDirect, which creates a canvas that is not owned by an SkSurface, so we must use std::unique_ptr there.

Simplified LockBits implementation to migrate away from SkCanvas::getTopDevice usage which is not really public anymore.

Some fixup to deal with gradient shader instantiation possibly returning null, so need to just draw transparent instead there to make some reftests pass.

Other bugbear: no more replayClips. So sanest idea here was to use what little public clip API there is left - isClipRect() and getDeviceClipBounds(). If the clip is a rectangle, we can just get the bounds and apply that to the CG context and call it good. For more complicated cases, we instead now have to use saveLayer, draw text to the new layer, then restore, which will blend back the layer with whatever complicated clips exist in the SkCanvas. At least for the common case (a simple clip rect), this should be fast, and relies only on public APIs.

Happy stuff - SkCanvas now has a way to tell saveLayer to just initialize the new layer with the contents of the previous layer, so all the annoyance of creating a handrolled CopyLayerImageFilter I can now just delete from the code.

Random little bits of API churn in terms of function names...
Attachment #8865117 - Flags: review?(mchang)
Assignee

Comment 8

2 years ago
Had to add in the glPolygonMode function to GLContext as Skia added this as a requirement for instantiating GL interface.
Attachment #8865118 - Flags: review?(mchang)
Assignee

Comment 9

2 years ago
Just some small fix-ups to deal with the above-mentioned issue that SkCanvas is no longer refcounted.
Attachment #8865119 - Flags: review?(mchang)
Assignee

Comment 10

2 years ago
More cleanup to deal with SkCanvas refcounting fallout. About the sanest thing we do here is to treat the ANPCanvas as the owner, which for our remaining use-cases (Flash plugin) should be safe.
Attachment #8865120 - Flags: review?(mchang)
Assignee

Comment 11

2 years ago
Yet more SkCanvas refcounting fallout. In this case, the document, which PrintTargetSkPDF already manages through sk_sp, owns the returned SkCanvas from beginPage, so we just directly point to it instead of having to use std::unique_ptr.
Attachment #8865122 - Flags: review?(mchang)
Assignee

Comment 12

2 years ago
Lots of reftest fuzzing to deal with the fact that Skia has done a lot of mucking around with path antialiasing since our last update. Fuzz all the things.
Attachment #8865236 - Flags: review?(mchang)
Comment on attachment 8865117 [details] [diff] [review]
fix Moz2d for Skia m59

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

With minor nits

::: gfx/2d/DrawTargetSkia.cpp
@@ +1092,5 @@
>  // SetupCGContext.
>  CGContextRef
>  DrawTargetSkia::BorrowCGContext(const DrawOptions &aOptions)
>  {
> +  bool needLayer = mCanvas->isClipEmpty() && !mCanvas->isClipRect();

Why do we need a layer if the clip is empty? How come the condition here isn't just if the clip is more complex than a rect?

@@ +1172,5 @@
>    CGContextRestoreGState(aCGContext);
> +
> +  bool needLayer = !mCanvas->isClipEmpty() && !mCanvas->isClipRect();
> +  if (needLayer) {
> +    if (mCG) {

do we need to release mCG here? Looks like in BorrowCGContext, we'll already release the context here.

@@ +1779,5 @@
>    }
>  
>    mCanvas->save();
>    mCanvas->setMatrix(SkMatrix::MakeTrans(SkIntToScalar(aDestination.x), SkIntToScalar(aDestination.y)));
> +  mCanvas->clipRect(SkRect::MakeIWH(aSourceRect.width, aSourceRect.height), SkClipOp::kReplace_deprecated);

Uhh, what's supposed to be the proper clip op? kReplace_deprecated doesn't sound very good. Why isn't this kIntersect like the other uses below?
Attachment #8865117 - Flags: review?(mchang) → review+
Attachment #8865118 - Flags: review?(mchang) → review+
Attachment #8865119 - Flags: review?(mchang) → review+
Comment on attachment 8865120 [details] [diff] [review]
fix Skia NPAPI for Skia m59

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

::: other-licenses/skia-npapi/SkANP.h
@@ +59,4 @@
>      }
>  
>      ~ANPCanvas() {
> +        delete skcanvas;

hoorah for deletion of raw pointer! Just make skcanvas here private please.
Attachment #8865120 - Flags: review?(mchang) → review+
Comment on attachment 8865122 [details] [diff] [review]
fix thebes for Skia m59

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

::: gfx/thebes/PrintTargetSkPDF.h
@@ +58,5 @@
>    // The stream that the SkDocument outputs to.
>    UniquePtr<SkWStream> mOStream;
>  
>    // The current page's SkCanvas and its wrapping DrawTarget:
> +  SkCanvas* mPageCanvas;

nit: Just say the documents own the mPageCanvas and mRefCanvas.
Attachment #8865122 - Flags: review?(mchang) → review+
Comment on attachment 8865236 [details] [diff] [review]
reftest fuzzing for update to Skia m59

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

<3 Fuzz
Attachment #8865236 - Flags: review?(mchang) → review+
Attachment #8865115 - Flags: review?(mchang) → review+
Assignee

Comment 17

2 years ago
(In reply to Mason Chang [:mchang] from comment #13)
> Comment on attachment 8865117 [details] [diff] [review]
> fix Moz2d for Skia m59
> 
> Review of attachment 8865117 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> With minor nits
> 
> ::: gfx/2d/DrawTargetSkia.cpp
> @@ +1092,5 @@
> >  // SetupCGContext.
> >  CGContextRef
> >  DrawTargetSkia::BorrowCGContext(const DrawOptions &aOptions)
> >  {
> > +  bool needLayer = mCanvas->isClipEmpty() && !mCanvas->isClipRect();
> 
> Why do we need a layer if the clip is empty? How come the condition here
> isn't just if the clip is more complex than a rect?

Typo. Meant !mCanvas->isClipEmpty(). Fixed that.

> @@ +1172,5 @@
> >    CGContextRestoreGState(aCGContext);
> > +
> > +  bool needLayer = !mCanvas->isClipEmpty() && !mCanvas->isClipRect();
> > +  if (needLayer) {
> > +    if (mCG) {
> 
> do we need to release mCG here? Looks like in BorrowCGContext, we'll already
> release the context here.

Since the mCanvas->restore() below it destroys the layer data the CG context referred to,
I try to make sure the CG context doesn't stick around after the layer goes bye-bye. That
way the context won't reused. Commented it as such.

> @@ +1779,5 @@
> >    }
> >  
> >    mCanvas->save();
> >    mCanvas->setMatrix(SkMatrix::MakeTrans(SkIntToScalar(aDestination.x), SkIntToScalar(aDestination.y)));
> > +  mCanvas->clipRect(SkRect::MakeIWH(aSourceRect.width, aSourceRect.height), SkClipOp::kReplace_deprecated);
> 
> Uhh, what's supposed to be the proper clip op? kReplace_deprecated doesn't
> sound very good. Why isn't this kIntersect like the other uses below?

CopySurface unfortunately has the spec'd behavior that it bypasses clip. For now it's not very easy or feasible
to change that until we do a more thorough investigation of what would break and how. For now Skia still supports
the Replace clip-op, but they are subtly hinting that they may not in the future. I'm just kicking the can down
the road on this one because solving it is a bit annoying without involvement from Skia guys possibly... or worse,
auditing every use of CopySurface in our codebase to not require the behavior. :/
Comment on attachment 8865116 [details] [diff] [review]
update moz.build for Skia m59

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

This seems reasonable. Since this isn't actually run during build time, I'll let ted do a post-commit review and we can fix up anything afterwards.
Attachment #8865116 - Flags: review?(jmuizelaar) → review+

Comment 19

2 years ago
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/af6f19870b2a
part 1 - update Skia source to m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d4ec7d38a00
part 2 - update moz.build for Skia m59. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/75fb0d9858a9
part 3 - fix Moz2d for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/fdbd5d281287
part 4 - fix thebes for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/026aadd76d06
part 5 - fix SkiaGL glue for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/3cb4bceb8d79
part 6 - fix layers for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/c691e2ab6a0c
part 7 - fix Skia NPAPI for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/f152be1fadb7
part 8 - reftest fuzzing for update to Skia m59. r=mchang
Posted patch More reftest fuzzing (obsolete) — Splinter Review
Attachment #8866358 - Flags: review?(lsalzman)
Whoops, typed 255 instead of 225 in the last patch. For the record you can see the reftest failures at https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=ebbcdaa5b5802ecd39624dd2acbdda8547b8384d&filter-searchStr=quantum
Attachment #8866358 - Attachment is obsolete: true
Attachment #8866358 - Flags: review?(lsalzman)
Attachment #8866362 - Flags: review?(lsalzman)
Assignee

Updated

2 years ago
Attachment #8866362 - Flags: review?(lsalzman) → review+

Comment 24

2 years ago
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f151e6ae195
Follow-up to fuzz a reftest when webrender is enabled. r=lsalzman
This patch makes Firefox unusable on - at least - OSX 10.12.4 due to abundant layout glitches in browser chrome and content.

I'm requesting a backout.
Flags: needinfo?(lsalzman)
Flags: needinfo?(bugmail)
(In reply to Mike de Boer [:mikedeboer] from comment #25)
> This patch makes Firefox unusable on - at least - OSX 10.12.4 due to
> abundant layout glitches in browser chrome and content.

Just to confirm, I'm seeing the same thing -- just spent some time bisecting locally and confirmed this is the bug where things start breaking.
We should back it out, but can we have a bit more detail on the STR?
STR:

./mach build
./mach run

on OSX.
If you don't see the glitches right away, try resizing the browser window and/ or opening a few tabs.

Pic: https://www.evernote.com/l/APsP8q1mHQtL8Ki0CMQA9tCsjRUJj1tHb48
Milan, Mike is in Toronto Commons if you want to have a hands on experience.
I'm mostly unrelated to this bug, so dropping needinfo on me.
Flags: needinfo?(bugmail)
Oops, that was only the followup patch.

Rest backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/761b3f55f10f3df16601ce62e4d15b0c558c0cfb
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Merged the backout to central so we can re-spin nightlies: https://hg.mozilla.org/mozilla-central/rev/ce2218406119c36a551e3faea4e192186ee46cc5
After saving a layer in Skia, some of the clip state gets reset in SkCanvas. Thus, the needLayer query in ReturnCGContext was always returning false since the SkCanvas::isClipRect() was returning true, meaning we'd never pop a layer.
Attachment #8866479 - Flags: review?(lsalzman)
Assignee

Updated

2 years ago
Flags: needinfo?(lsalzman)
Attachment #8866479 - Flags: review?(lsalzman) → review+

Comment 36

2 years ago
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ded74baeaf2
part 1 - update Skia source to m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b2683a840f3
part 2 - update moz.build for Skia m59. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/abffc78e1cf6
part 3 - fix Moz2d for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/7134018d3724
part 4 - fix thebes for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/84fd6b2bfaf6
part 5 - fix SkiaGL glue for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/b567eb8d5d39
part 6 - fix layers for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/a874592cff0a
part 7 - fix Skia NPAPI for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/ebc9a119bcf8
part 8 - reftest fuzzing for update to Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/64c183fa4dd3
clobber for Skia update. r=me
https://hg.mozilla.org/integration/mozilla-inbound/rev/2477a27a95a9
Follow-up to fuzz a reftest when webrender is enabled. r=lsalzman
https://hg.mozilla.org/integration/mozilla-inbound/rev/1fcd9ee5ccb5
Properly push/pop skia saved layers during borrow/return cg context. r=lsalzman
The inevitable question here: how could Firefox break this badly, but still have these changesets merged into m-c? IOW: can we have a test that goes orange when the UI goes bonkers?
I _know_ that this happens _very_ rarely, but seeing this, bisecting, requesting backout and the multiple clobber builds did destroy a rather high amount of hours I couldn't spend on Photon. (Apology for the perhaps grumpy tone; I'm a wee bit tired ;-) )
Reporter

Comment 38

2 years ago
That's a completely fair question, Mike. And one that I was intending to ask myself had you not beat me to it! Lee, is there any kind of test we could add that would have caught the issue that led to the backout?
Flags: needinfo?(lsalzman)
Assignee

Comment 39

2 years ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #38)
> That's a completely fair question, Mike. And one that I was intending to ask
> myself had you not beat me to it! Lee, is there any kind of test we could
> add that would have caught the issue that led to the backout?

I myself assumed that since this had manage to survive the entire test suite unscathed on try several times over, where we already have a bunch of mochitests and reftests for browser chrome, that, well, it was working. Unfortunately, I'm not familiar with the specifics of the macOS widget code enough to understand why none of these possible failures were getting triggered at all.

Mason or Markus, do you have possibly ideas on why this might be the case? This is definitely somewhat of a mystery.
Flags: needinfo?(mstange)
Flags: needinfo?(mchang)
Flags: needinfo?(lsalzman)
have to backout this changes because they block a m-i to m-c merge like

erging layout/reftests/image-element/reftest.list
merging layout/reftests/svg/reftest.list
warning: conflicts while merging editor/reftests/reftest.list! (edit, then use 'hg resolve --mark')
warning: conflicts while merging layout/reftests/box-shadow/reftest.list! (edit, then use 'hg resolve --mark')
warning: conflicts while merging layout/reftests/bugs/reftest.list! (edit, then use 'hg resolve --mark')
warning: conflicts while merging layout/reftests/forms/select/reftest.list! (edit, then use 'hg resolve --mark')
warning: conflicts while merging layout/reftests/image-element/reftest.list! (edit, then use 'hg resolve --mark')
1468 files updated, 8 files merged, 371 files removed, 5 files unresolved
use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to abandon
Flags: needinfo?(lsalzman)

Comment 42

2 years ago
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2aa2f6aa8053
part 1 - update Skia source to m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/cbb70e16727b
part 2 - update moz.build for Skia m59. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/306f65d0a9eb
part 3 - fix Moz2d for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/7dc2a529c1a9
part 4 - fix thebes for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb088b241376
part 5 - fix SkiaGL glue for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d02032c3c3b
part 6 - fix layers for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/a97b1c46e56e
part 7 - fix Skia NPAPI for Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/59b3cd42b4b5
clobber for Skia update. r=me
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ba1d4e0178a
part 8 - reftest fuzzing for update to Skia m59. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ebe08d1aa92
Follow-up to fuzz a reftest when webrender is enabled. r=lsalzman
https://hg.mozilla.org/integration/mozilla-inbound/rev/e06077091df1
Properly push/pop skia saved layers during borrow/return cg context. r=lsalzman
Assignee

Comment 43

2 years ago
(In reply to Carsten Book [:Tomcat] from comment #40)
> have to backout this changes because they block a m-i to m-c merge like
> 
> erging layout/reftests/image-element/reftest.list
> merging layout/reftests/svg/reftest.list
> warning: conflicts while merging editor/reftests/reftest.list! (edit, then
> use 'hg resolve --mark')
> warning: conflicts while merging layout/reftests/box-shadow/reftest.list!
> (edit, then use 'hg resolve --mark')
> warning: conflicts while merging layout/reftests/bugs/reftest.list! (edit,
> then use 'hg resolve --mark')
> warning: conflicts while merging layout/reftests/forms/select/reftest.list!
> (edit, then use 'hg resolve --mark')
> warning: conflicts while merging layout/reftests/image-element/reftest.list!
> (edit, then use 'hg resolve --mark')
> 1468 files updated, 8 files merged, 371 files removed, 5 files unresolved
> use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to
> abandon

Le sigh, merged and fixed.
Flags: needinfo?(lsalzman)
(In reply to Lee Salzman [:lsalzman] from comment #39)
> Mason or Markus, do you have possibly ideas on why this might be the case?
> This is definitely somewhat of a mystery.

I'm surprised by this. Our reftests don't use a lot of -moz-appearance, so maybe that's how this bug evaded detection.
It should be possible to write a test for this particular bug.
Flags: needinfo?(mstange)
Comment on attachment 8865116 [details] [diff] [review]
update moz.build for Skia m59

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

If this was part of the build I would be pickier, but as an import script it's fine. This might be useful to inform the work in bug 1336429, as well.

::: gfx/skia/generate_mozbuild.py
@@ +155,5 @@
> +          ('none', 'none', [':none'])]
> +
> +  opt_sources = {}
> +  for key, cpu, deps in cpus:
> +    subprocess.check_output('cd skia && bin/gn gen out/{0} --args=\'target_cpu="{1}"\''.format(key, cpu), shell=True)

FWIW, instead of having `cd skia` in there and doing string interpolation you can do:
subprocess.check_output(['bin/gin', 'gen', 'out/' + key, '--args=target_cpu="%s"' % cpu], cwd='skia')

Also if you're not using the output of the command, `subprocess.check_call` may be more appropriate.
Attachment #8865116 - Flags: review?(ted) → review+

Updated

2 years ago
See Also: → 1364560
Awesome, looks like this patch has a performance win:

== Change summary for alert #6531 (as of May 10 2017 02:16 UTC) ==

Improvements:

 14%  tcanvasmark summary osx-10-10 opt e10s     6,369.58 -> 7,279.08

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=6531
Posted file Ugly WIP Test
Ugly WIP Xul test that kinda mimics the top tab bar. I can only reproduce this with XUL elements. The weird thing is, I can repro this quite easily if I manually move my mouse between the elements. However, sending mouse events or clicking the events programmatically doesn't show the bug.
Flags: needinfo?(mchang)
Depends on: 1364691
Depends on: 1364840

Updated

2 years ago
Depends on: 1364851

Updated

2 years ago
Depends on: 1364774
Minimal test case of the chrome UI tab bar. Since it's kinda random like the original bug, we have to iterate through the tabs a couple of times to guarantee that the bug surfaces.
Attachment #8867895 - Flags: review?(lsalzman)
Assignee

Updated

2 years ago
Attachment #8867895 - Flags: review?(lsalzman) → review+

Comment 51

2 years ago
Pushed by mchang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e527334fbcd
OS X XUL UI test to check for CG Borrow/Return CGContext r=lsalzman

Updated

2 years ago
See Also: → 1368720
Depends on: 1369949
Reporter

Updated

2 years ago
Depends on: 1378130

Updated

2 years ago
Depends on: 1395127
Assignee

Updated

2 years ago
Depends on: 1387079
Assignee

Updated

2 years ago
Depends on: 1404128

Updated

Last year
Depends on: 1433848
You need to log in before you can comment on or make changes to this bug.