Re: Proposal to remove unnecessary [type] attributes on script tags in mozilla-central

[Felipe G]

> I'm writing to the list in order to:
> 2) See if there are any general best-practices for getting this type of
> change reviewed / landed. For example, should we prefer separate commits /
> reviews per directory, or does a single tree-wide commit make sense?
>
> Please include the string "# ignore-this-changeset" in the commit summary
so that it can be skipped on the blame view in searchfox
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Improving our usage of Bugzilla

[Felipe G]

Does performance work count as "enhancement" or "task"?
On one hand, it's not strictly refactoring.. On the other hand, it is not
development of a new feature, per the proposed description of enhancement.

On Tue, Mar 12, 2019 at 3:20 PM Onno Ekker  wrote:

> On 12/03/2019 18:59, Sylvestre Ledru wrote:
> > Le 12/03/2019 à 17:48, Andrew McCreight a écrit :
> >> 
> >> Secondly, to bikeshed a little, could there be some name besides
> >> "task" for
> >> that third category? Like I said above, everything we work as
> >> developers is
> >> a developer task. "Refactor" seems like a clearer name, though maybe
> >> it is
> >> a little limiting. "Side grade"? :)
> >
> > This is more than just refactoring. It is more "as an engineer, here is
> > what I have to do".
>
> Maybe call it "Engineering"? "Maintenance"?
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Skipping changesets when viewing blame

[Felipe G]

If you ran `mach bootstrap` or `mach vcs-setup` in the last month, you
should already have a new hg command alias called `smart-annotate`, which
runs `hg annotate` while ignoring some predefined changesets.

The primary use case is to ignore (semi-)automatic code-formatting changes.
The list of changesets to ignore comes from two sources:
 - any commit that contains the string "ignore-this-changeset" in the
summary (commit message) will be ignored automatically
 - other commits that doesn't contain that string can be added to the
.hg-annotate-ignore-revs
 file

Besides the command itself, there's also the "ignored_changesets" revset
alias that you can use as you wish in other commands (e.g., `hg log -r
ignored_changesets` will show you all the csets to be ignored)

Sylvestre has included the "ignore-this-changeset" string in the tree-wide
C++ style change patch, and we've gone and back-filled the
.hg-annotate-ignore-revs file with previous eslint and clang-format changes.

Happy New Year!
Felipe

(This work was tracked on bugs 1508002
 and 1508324
)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Upcoming changes to our C++ Coding Style

[Felipe G]

 On Thu, Nov 22, 2018 at 3:07 PM Ehsan Akhgari 
wrote:

> Felipe, gps and I talked a bit about adding a similar
> .hg-blame-ignore-revs file in the tree which can contain Mercurial sha1
> changeset IDs for the rewrite commits to be skipped over, and people would
> be able to use the file through an alias that can be configured in ~/.hgrc
> (possible to set it up via ./mach bootstrap).  Not sure if Felipe's
> investigations have lead to results yet.
>

Indeed they have. I'm working on this in two separate bugs:

https://bugzilla.mozilla.org/show_bug.cgi?id=1508002 is implementing the
same functionality in mercurial from`git hyper-blame`, and
https://bugzilla.mozilla.org/show_bug.cgi?id=1508324 will be used to
generate the list of past changesets

Hope to have these wrapped up soon, and I'll write about them once they're
done. One thing to mention in advance is that gps asked to not use the word
"blame" as the Mercurial project doesn't like the negative connotations
that it brings, so I'm leaning towards `hg smart-annotate` and "#
ignore-changeset" for the string to be included in the commit message. Let
me know if anyone has thoughts about these (or leave comments in the bug)

Felipe
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Fission MemShrink Newsletter #1: What (it is) and Why (it matters to you)

[Felipe G]

>
>
>
> >Also note that dealing with the "importance" of a page is not just a
> >matter of visibility and focus. There are other factors to take into
> >account such as if the page is playing audio or video (like listening to
> >music on YouTube), if it's self-updating and so on.
>
> Absolutely
>

We should think about how we can make different performance and memory
trade-offs for processes that are hosting top-level frames and processes
hosting 3rd-party subframes

>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to Ship: Enterprise Policies

[Felipe G]

Summary: The Enterprise Policies manager is a feature to allow
administrators to configure and lock some aspects of Firefox across several
machines in an enterprise deployment.
We intend to enable it right now so that it rolls out normally through the
beta cycle, where we'll gather feedback from system administrators.
The target release for this feature is ESR 60.

The policy engine itself went through architectural and security review,
and each individual policy has been reviewed by an owner/peer of the
specific feature that it controls, with automated tests and manual QA being
done for every policy.

The feature will be available on Release but, to prevent abuse, a few
critical policies will only be allowed to be used on ESR installations.

When the feature is in use, a notice (to the end user) in about:preferences
will be displayed, as well as en entry in about:support.

Related bugs:
- Bug to enable it by default: https://bugzilla.mozilla.org/
show_bug.cgi?id=1433172
- Tracking bug for all the tasks related to shipping it:
https://bugzilla.mozilla.org/show_bug.cgi?id=1433173
- Tracking bug for all the policies wanted for 60:
https://bugzilla.mozilla.org/show_bug.cgi?id=1428920


Platform: Desktop
Target release: Firefox 60
Preference: None
DevTools bug: N/A
web-platform-tests: N/A
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Changes to tab min-width

[Felipe G]

I like having this option back, as I know this was a feature that a lot of
people liked in the past.  (even though I'm personally ok with the 100px
width)

I've been trying to use the new default (50px) for a couple of hours, and
it felt surprisingly unusable, in a way that I couldn't quite figure out
why. After a while, I think I've reached the explanation:

The purpose of overflowing tabs is to prevent them to go shrink to unusable
sizes.. Even at 50px, I have enough tabs open to make it overflow. So it
overflows, but the tabs won't grow back to something more usable, so what
happens is that I have a tab strip full of unreadable tabs, and it still
scrolls  (i.e., there was no benefit in the smaller min-width, only
downsides)

It would be nice if the tabs would grow again a little bit when overflow
happens, but that is probably too daunting visually. So I know this is a
no-go.

Playing with the value, I actually really liked 80px, and I can see that
winning in favor of 100px.  We could perhaps go as low as 75, but IMHO that
would be the limit.

my 2 cents
Felipe

On Tue, Oct 3, 2017 at 5:36 PM, Jeff Griffiths 
wrote:

> Hi!
>
> tl;dr we changed the default pixel value at which we overflow tabs,
> and I want your feedback.
>
> We just added a change to m-c[1] that does to things:
>
> 1. it reintroduces an old preference 'browser.tabs.tabMinWidth' that
> contains a pixel value that controls the minimum width of a tab.
>
> 2. it sets the default value of the tab to 50, previously this value
> was hard-coded at 100.
>
> Work is being tracked in https://bugzilla.mozilla.org/
> show_bug.cgi?id=1404465
>
> We did this based on some early feedback from a few different sources
> that people coming from chrome ( or in some cases, existing users )
> thought that the Firefox behaviour of scrolling the tabstrip was
> off-putting. We looked into this and I generally agree with the
> comments: chrome's "infinite tabs visible" approach results in a much
> higher usable/visible tab count in a given window than ours does. This
> change puts us roughly at par.
>
> To put this in numbers:
>  * in chrome I can open ~ 24 tabs before the tabstrip's usability is
> degraded a lot
>  * in current firefox, I can open ~ 12 tabs before tabstrip scrolling
> kicks in
>  * with this change applied I can open 25 tabs with the pref value set to
> 50px
>
> ( Caveats: this was on the built-in display on my Macbook Pro with the
> default theme, your mileage may vary, etc )
>
> I want feedback on this change from these lists, and will also be
> looking for feedback from the original sources of this complaint. In
> particular:
>
> 1. do you prefer the existing behaviour or the new behaviour?
> 2. if you prefer a value for this pref different than 50 or 100, what
> is it? Why?
>
> One aspect that I would like to stress about this change: most
> existing Firefox users will never see it, because they are unlikely to
> open m,ore than 10 tabs at any one time. So what we are really talking
> about is a change that will trade being able to see more tabs vs being
> able to read more text in each tab title.
>
> Moving forward there are a few different options:
>
> 1. uplifting this change into 57 ( possibly with a different default
> value ) If we think the patch has a generally positive effect and no
> downsides, we may decide to uplift into 57 Beta and let it ride the
> trains.
>
> 2. keeping the change in 58, possibly with a different value.
>
> 3. keeping the change in 58, preserving the current setting of 100px
> and providing an alternate pref ( probably a toggle or predefined
> values ) for "skinnier" tabs.
>
> Longer term I intend to propose a more in-depth study of tab behaviour
> among different user segments and assess different strategies for
> heavier tab users including things like horizontal tab scaling,
> vertical tabs, etc. I can't see that happening before Q1 next year.
>
> cheers, Jeff
>
> [1] https://hg.mozilla.org/integration/autoland/rev/a75e0386aad8
> ___
> firefox-dev mailing list
> firefox-...@mozilla.org
> https://mail.mozilla.org/listinfo/firefox-dev
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: disabled non-e10s tests on trunk

[Felipe G]

I ran some scripts that I had to find out tests that are *fully* disabled
on e10s, and posted the results to
https://bugzilla.mozilla.org/show_bug.cgi?id=1376934

In summary:
mochitest-plain: 49 tests
browser-chrome: 15 tests
devtools: 86 tests

Note that the script evaluates the skip-if condition for each test, so it's
able to not count tests such as "skip=if e10s && debug" as fully disabled.

On Wed, Aug 9, 2017 at 7:35 AM, Gabor Krizsanits 
wrote:

> On Wed, Aug 9, 2017 at 3:36 AM, Boris Zbarsky  wrote:
>
> >
> > Hmm.  Do we load about:blank from the url bar in a content process?
> >
> >
> Yes.
>
> I agree, I find it annoying too that we have to rely on
> MOZ_DEBUG_CHILD_PROCESS
> or MOZ_DEBUG_CHILD_PAUSE and that I have to be clever all the time about
> how to hit the right process at the right time with the debugger. I never
> switched back to non-e10s though since I don't trust that everything will
> work the same and I don't think that should be the solution. Switching back
> to single content process for debugging should come with less side effects
> though... Also, this is not just an e10s/e10s-multi related issues we're
> adding all kind of processes (extension/gpu/plugin/etc.).
>
> I didn't file a bug about this but I've been trying to find a decent
> solution for it, but it seems like it's not trivial in any debugger (msvc,
> gdb, lldb). Or maybe I was just hoping to find something better than what
> seems to be achievable. Anyway, let's start with the bug: Bug 1388693.
>
> Gabor
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: ESlint rule 'no-arbitrary-setTimeout' enabled on xpcshell tests

[Felipe G]

I'll note that requestFlakyTimeout is only enabled for mochitest-plain at
the moment: http://searchfox.org/mozilla-central/source/testing/
mochitest/tests/SimpleTest/SimpleTest.js#666
So browser-chrome / a11y / chrome tests are still able to use non-0
timeouts.

Cheers,
Felipe

On Fri, Jul 28, 2017 at 12:48 PM, Andrew Halberstadt <
ahalberst...@mozilla.com> wrote:

> As part of a larger effort to reduce oranges, we are starting to lint our
> tests for common causes of intermittent failures. One low-hanging fruit is
> preventing setTimeout with an arbitrary value (aka non-zero) as opposed to
> waiting for an appropriate event. The mochitest harness already prevents
> this in the harness itself (SimpleTest.requestFlakyTimeout), so this rule
> is only enabled on xpcshell tests for now.
>
> If you need to use a flaky setTimeout for some reason, you can disable the
> rule at the directory level, file level or line level:
> http://eslint.org/docs/user-guide/configuring#configuring-rules
>
> It has been disabled in the following files due to pre-existing violations:
> http://searchfox.org/mozilla-central/search?q=eslint-disable
> +mozilla%2Fno-arbitrary-setTimeout
>
> Let me know if you think this should be enabled on any other test suites.
> -Andrew
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Profiling nightlies on Mac - what tools are used?

[Felipe G]

The Activity Monitor has a built-in process sampling tool that is very
handy and I use it every now and then.

On Mon, Jun 19, 2017 at 7:40 PM, Kearwood Kip Gilbert 
wrote:

> I would add to this Apple’s “OpenGL Profiler”:
>
> https://developer.apple.com/library/content/technotes/tn2178
>
> Cheers,
> - Kip
>
> From: Bobby Holley
> Sent: June 19, 2017 3:08 PM
> To: Chris Cooper
> Cc: dev-platform@lists.mozilla.org
> Subject: Re: Profiling nightlies on Mac - what tools are used?
>
> Instruments is the big one that I'm aware of.
>
> On Mon, Jun 19, 2017 at 3:03 PM, Chris Cooper  wrote:
>
> > Hey all,
> >
> > The build peers are looking to change the way that nightlies are created
> > on Mac as we switch to cross-compilation. Specifically, we're looking at
> > stripping the nightlies to avoid an as-of-yet undiagnosed performance
> > discrepancy vs native builds[1], but also to make the nightly
> configuration
> > match what we ship on beta/release (stripped).
> >
> > Of course, stripping removes the symbols, and while we believe we have a
> > solution for re-acquiring symbols that works for the Gecko Profiler, we
> > realize
> > that people out there may be using other profiling tools.
> >
> > If you profile on Mac, now is your chance to speak up. What other
> > profiling tools do you use that we should be aware of?
> >
> > cheers,
> > --
> > coop
> >
> > 1. https://bugzilla.mozilla.org/show_bug.cgi?id=1338651
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> >
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to implement: Dynamic blocklist to disallow plugins triggered from specified 3rd-party domains

[Felipe G]

Summary:

This intent covers adding a dynamic blocklist (to be stored/updated through
Shavar)
to disallow plugins running on specific domains when they are in a
3rd-party context
(e.g. a 3rd party iframe).

Motivation:

Flash and other plugins are still a primary target for malware authors and
distributors. In order to mitigate the risk of large-scale attacks, we're
going
to ask sites that are commonly loaded across the internet to disallow
any plugins running from their domain.

Bugs:
https://bugzilla.mozilla.org/show_bug.cgi?id=1277876

Link to standard: -

Platform coverage: all

Estimated or target release: Firefox 50

Preference behind which this will be implemented: no pref.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Checkin-needed requests - Please include complete information in the commit message :)

[Felipe G]

Is there a way to make the checkin-needed flag generate a template comment
(like the approval-* ones do) with something like this? (Or encourage
people to use the per-patch checkin? flag)

"""
Has this patch been through try? [ Yes / No, I believe it's not necessary ]
Does this patch contain the correct author / commit message? [ Yes
(preferred) / No, but I'm providing it here: ]
Are there any other dependencies that should be landed together? [ Yes, ...
/ No ]
"""

Probably just asking if the information is present will reduce the number
of requests made without it

On Fri, Jul 8, 2016 at 10:47 AM, Ryan VanderMeulen  wrote:

> FWIW, there's also an MDN page that documents a lot of this as well:
>
> https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
>
> -Ryan
>
>
> On 7/8/2016 2:32 AM, Carsten Book wrote:
>
>> Hi,
>>
>> someone might not know that doing checkins for checkin-needed request is
>> not automated yet and completely a fully human task :) (no we Sheriffs are
>> not bots ;)
>>
>> It would help us a lot if a checkin needed request would contain complete
>> Author/Patch information like:
>>
>>
>>- Author (use the information from their Bugzilla account if needed)
>>with Name *and *Emailadress.
>>- Bug number
>>- Commit message (keeping in mind that the commit message should be a
>>brief description of what the patch is *doing*)
>>   - Format should be something like "Bug 123456 - Add a null check to
>>   XYZ to avoid a crash. r=somebody"
>>
>>
>> And also if there is a specific sequence/dependency you want to checkin
>> the
>> patches it would help also a lot  if you could make a short comment in the
>> Bug like please checkin part x then patch y or like first bug 123 then
>> this
>> bug and then bug 8910.
>>
>> This would help us a lot :)
>>
>> Thanks!
>>
>> - Tomcat
>>
>>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Testing Wanted: APZ Scrollbar dragging

[Felipe G]

Hey Jonas,

this second issue that you're seeing is tracked at
https://bugzilla.mozilla.org/show_bug.cgi?id=1257862

Cheers,
Felipe

On Tue, Mar 29, 2016 at 5:55 PM, Jonas Sicking  wrote:

> Hi Benoit,
>
> There's two problems that I've seen related to scrolling, and so
> possibly the result of APZ.
>
> The most obvious one is that if the child process is busy doing
> things, then the scrollable area seems "sticky". When you start
> scrolling there's a noticeable delay before the page starts scrolling.
>
> My guess is that we send some message to the child process when
> scrolling is initiated and don't start scrolling until the child
> process responds (possibly with some form of timeout)?
>
> This isn't noticeable on the example page that you provided, possibly
> because the child process is quite responsive before we start the
> heavy painting operations.
>
> The other problem, which is more serious, is that every now and then
> the browser seems to get into a state where I can't use the trackpad
> at all to do scrolling. The scroll bar is displayed, as is common when
> using the trackpad to scroll, but no scrolling actually happens.
>
> The only way I can scroll when the browser is in this state is to
> hover the scrollbar and click below the scroll thumb, which enables
> scrolling one page at a time.
>
> The only way to get out of the state is by restarting the browser.
>
> Sadly this happens very infrequently, so I haven't been able to create
> steps to reproduce, nor seen any pattern in where it triggers.
>
> / Jonas
>
>
> On Wed, Feb 17, 2016 at 10:35 AM, Benoit Girard 
> wrote:
> > Currently APZ does not cause scrollbar initiated scrolling to be async.
> > I've been working in fixing this and I'd like some help testing it out
> > before enabling it on Nightly. If you're interested please flip
> > 'apz.drag.enabled' to true and restart. If you find any issue please make
> > it block https://bugzilla.mozilla.org/show_bug.cgi?id=1211610.
> >
> > I've got a test page here to confirm that it's turned on properly:
> > http://people.mozilla.org/~bgirard/scroll_no_paint.html
> >
> > Scrolling on this page is slow in FF because it hits a performance bugs
> in
> > layer construction code. However with APZ the scrolling is smooth and a
> bit
> > checkerboard-y instead. If you have 'layers.acceleration.draw-fps' set
> you
> > should notice the left most FPS counter at 60 FPS while the middle
> counter
> > will be much lower. This is APZ handling the scroll.
> >
> > Happy Testing,
> > Benoit
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to enable e10s by default when running tests locally

[Felipe G]

Yeah, that sounds like another good outcome of replacing --e10s with
--disable-e10s.

On Thu, Mar 24, 2016 at 3:59 PM, Ehsan Akhgari <ehsan.akhg...@gmail.com>
wrote:

> On Thu, Mar 24, 2016 at 2:10 PM, Felipe G <fel...@gmail.com> wrote:
>
>> Yeah, --e10s enables e10s in the browser for mochitest-chrome.  However,
>> the test harness is a .xul file opened in a tab, and that runs that tab as
>> non-remote, so for most tests it ends up testing the same thing as not
>> using --e10s. Other tabs and/or windows opened manually by the test would
>> be remote.
>>
>
> D'oh, I have been working on this stuff and I didn't realize that's the
> case (I was operating as if a passing mochitest-chrome when run in e10s
> actually works with e10s).  That's extremely confusing.  :(  Should we
> refuse to accept --e10s when running mochitest-chrome to help avoid this
> confusion?
>
>
>>
>> On Thu, Mar 24, 2016 at 3:05 PM, Ehsan Akhgari <ehsan.akhg...@gmail.com>
>> wrote:
>>
>>> On 2016-03-24 1:25 PM, Andrew McCreight wrote:
>>> > One potential sticking point is mochitest-chrome. It currently does not
>>> >> get run in e10s in CI, so local configuration should mirror this.
>>> >> However, since --e10s is being removed, this means it would be
>>> >> impossible to run mochitest-chrome in e10s without modifying source
>>> >> files. Maybe this just needs some argparse hackery.
>>> >>
>>> >
>>> > My impression was that mochitest-chrome doesn't actually run with e10s,
>>> > even when you specify the flag. Is that not correct?
>>>
>>> No.  You currently can force it to run in e10s with --e10s like other
>>> mochitest variants.
>>>
>>> ___
>>> firefox-dev mailing list
>>> firefox-...@mozilla.org
>>> https://mail.mozilla.org/listinfo/firefox-dev
>>>
>>
>>
>
>
> --
> Ehsan
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to enable e10s by default when running tests locally

[Felipe G]

Yeah, --e10s enables e10s in the browser for mochitest-chrome.  However,
the test harness is a .xul file opened in a tab, and that runs that tab as
non-remote, so for most tests it ends up testing the same thing as not
using --e10s. Other tabs and/or windows opened manually by the test would
be remote.


On Thu, Mar 24, 2016 at 3:05 PM, Ehsan Akhgari 
wrote:

> On 2016-03-24 1:25 PM, Andrew McCreight wrote:
> > One potential sticking point is mochitest-chrome. It currently does not
> >> get run in e10s in CI, so local configuration should mirror this.
> >> However, since --e10s is being removed, this means it would be
> >> impossible to run mochitest-chrome in e10s without modifying source
> >> files. Maybe this just needs some argparse hackery.
> >>
> >
> > My impression was that mochitest-chrome doesn't actually run with e10s,
> > even when you specify the flag. Is that not correct?
>
> No.  You currently can force it to run in e10s with --e10s like other
> mochitest variants.
>
> ___
> firefox-dev mailing list
> firefox-...@mozilla.org
> https://mail.mozilla.org/listinfo/firefox-dev
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Status: e10s on 45, 46, and 47

[Felipe G]

On Mon, Mar 7, 2016 at 2:27 PM, Ehsan Akhgari 
wrote:

> Hi Jim,
>
> On 2016-03-07 10:25 AM, Jim Mathies wrote:
> > Users involved in this experiment will:
> >
> > - not have addons installed
> > - will not have used accessibility recently (including touch screen
> users)
> > - will not be using rtl builds
>
> Out of curiosity, what's the reason for excluding the RTL locales in
> this experiment?  Is there a difference in e10s between LTR and RTL
> locales?
>

It's due to bugs https://bugzilla.mozilla.org/show_bug.cgi?id=1033488 and
https://bugzilla.mozilla.org/show_bug.cgi?id=1033483 which are not fixed
yet and have been marked as not blockers for a initial rollout.  My
understanding is that if a fix for them lands in time for the first rollout
we can remove the RTL exclusions.  But it would need some baking time to
make sure that no other bugs are concealed due to this detection not
working properly at the moment.

Cheers,
Felipe


>
> Thanks,
> Ehsan
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

e10s tests

[Felipe G]

(cross-posted to fx-dev and dev.platform)

As we drive towards shipping e10s, we are working on making sure that our
tests work with e10s. As you already know, there's a number of tests that
are disabled from running on e10s, and we need to enable them. We've
created a spreadsheet that lists every test that is disabled from running
on e10s in order to track the remaining work to do.

https://docs.google.com/spreadsheets/d/10UeyRoiWV2HjkWwAU51HXyXAV7YLi4BjDm55mr5Xv6c/edit?usp=sharing

Please take a look at the list and help in any way you can. Feel free to
add your name to any test that you pick to fix, or add comments about tests
that you have knowledge about. Even a comment describing what the test is
testing and/or what it takes to make it pass on e10s helps.  For tests that
can't be fixed in the timeframe of shipping e10s, we will consider manual
testing of the things that they are supposed to test. But of course that's
a stop gap solution until they can be properly fixed.

Blake and I will be reaching out to module owners and peers to find owners
for tests. It is each team's responsibility to ensure that their features
are properly tested under e10s! So do help and spread the word around.

There's a wiki page with tips on how to convert tests:
https://wiki.mozilla.org/Electrolysis/e10s_test_tips, and Blake has also
written about it recently:
https://blog.mozilla.org/mrbkap/2015/12/10/converting-a-test-to-be-e10s-compatible/
.

Ping me, mrbkap or anyone in #e10s if you have questions!

Felipe
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: How to efficiently walk the DOM tree and its strings

[Felipe G]

Chrome imports a JS script into the webpage and this script does all the
translation work.

Felipe

On Mon, Mar 3, 2014 at 4:31 PM, Jeff Muizelaar jmuizel...@mozilla.comwrote:


 On Mar 3, 2014, at 2:28 PM, Felipe G fel...@gmail.com wrote:

  Hi everyone, I'm working on a feature to offer webpage translation in
  Firefox. Translation involves, quite unsurprisingly, a lot of DOM and
  strings manipulation. Since DOM access only happens in the main thread,
 it
  brings the question of how to do it properly without causing hank.

 What does Chrome do?

 -Jeff

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: How to efficiently walk the DOM tree and its strings

[Felipe G]

Thanks for the feedback so far!

If I go with the clone route (to work on the snapshot'ed version of the
data), how can I later associate the cloned nodes to the original nodes
from the document?  One way that I thought is to set a a userdata on the
DOM nodes and then use the clone handler callback to associate the cloned
node with the original one (through weak refs or a WeakMap).  That would
mean iterating first through all nodes to add the handlers, but that's
probably fine (I don't need to analyze anything or visit text nodes).

I think serializing and re-parsing everything in the worker is not the
ideal solution unless we can find a way to also keep accurate associations
with the original nodes from content. Anything that introduces a possibly
lossy data aspect will probably hurt translation which is already an
innacurate science.


On Tue, Mar 4, 2014 at 6:26 AM, Andrew Sutherland 
asutherl...@asutherland.org wrote:

 On 03/04/2014 03:13 AM, Henri Sivonen wrote:

 It saddens me that we are using non-compliant ad hoc parsers when we
 already have two spec-compliant (at least at some point in time) ones.


 Interesting!  I assume you are referring to:
 https://github.com/davidflanagan/html5/blob/master/html5parser.js

 Which seems to be (explicitly) derived from:
 https://github.com/aredridel/html5

 Which in turn seems to actually includes a few parser variants.

 Per the discussion with you on https://groups.google.com/d/
 msg/mozilla.dev.webapi/wDFM_T9v7Tc/Nr9Df4FUwuwJ for the Gaia e-mail app
 we initially ended up using an in-page data document mechanism for
 sanitization.  We later migrated to using a worker based parser.  There
 were some coordination hiccups with this migration (
 https://bugzil.la/814257) and some time B2G time-pressure so a
 comprehensive survey of HTML parsers did not happen so much.

 While we have a defense-in-depth strategy (CSP and iframe sandbox should
 be protecting us from the worst possible scenarios) and we're hopeful that
 Service Workers will eventually let us provide nsIContentPolicy-level
 protection, the quality of the HTML parser is of course fairly important[1]
 to the operation of the HTML sanitizer.  If you'd like to bless a specific
 implementation for workers to perform streaming HTML parsing or other some
 other explicit strategy, I'd be happy to file a bug for us to go in that
 direction.  Because we are using a white-list based mechanism and are
 fairly limited and arguably fairly luddite in what we whitelist, it's my
 hope that our errors are on the side of safety (and breaking adventurous
 HTML email :), but that is indeed largely hope.  Your input is definitely
 appreciated, especially as it relates to prioritizing such enhancements and
 potential risk from our current strategy.

 Andrew


 1: understatement

 ___
 dev-platform mailing list
 dev-platform@lists.mozilla.org
 https://lists.mozilla.org/listinfo/dev-platform

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: How to efficiently walk the DOM tree and its strings

[Felipe G]

The actual translation needs to happen at once, but that's ok if I can work
in the chunks incrementally, and only when everything is ready I send it
off to the translation service.  What I need to find then is a good (and
fast) partitioning algorithm that will give me a list of several blocks to
translate. A CSS block is a good start but I need something more detailed
than that for some of these reasons:

- I can't skip invisible or display:none nodes because websites have
navigation menus and etc. that have text on them and need to be translated
(I don't know what's the correct definition of CSS block that you mention
to know if it covers this or not)
- In direct opposition of the first point, I can't blindly just consider
all nodes (including invisible ones) with text content on them because
websites have script, style... tags in the body which should be skipped


Also, some other properties that I'd like this algorithm to have:

- It would be nice if it can treat a ul lia/li libli /ul as one
individual block instead of one-per-li   (and other similar constructs)
-- [not a major req]

- It should only give me blocks that have useful text content inside to be
translated. For example, for sites with a lot of divdivdivdiv (or
worse with tabletbodytrtdtable.. ad infinitum) nesting, I'm only
interested in the blocks that have actual text content on them (which can
probably be defined as with at least one non-whitespace-only child text
node).


The more junk (useless nodes) that this algorithm can skip, the better.
Then I imagine we'll be good in performance if I implement it in C++ and
have all other handling and further filtering be done in JS, one chunk at a
time.


Felipe



On Tue, Mar 4, 2014 at 6:02 PM, Robert O'Callahan rob...@ocallahan.orgwrote:

 On Wed, Mar 5, 2014 at 8:47 AM, Felipe G fel...@gmail.com wrote:

 If I go with the clone route (to work on the snapshot'ed version of the
 data), how can I later associate the cloned nodes to the original nodes
 from the document?  One way that I thought is to set a a userdata on the
 DOM nodes and then use the clone handler callback to associate the cloned
 node with the original one (through weak refs or a WeakMap).  That would
 mean iterating first through all nodes to add the handlers, but that's
 probably fine (I don't need to analyze anything or visit text nodes).

 I think serializing and re-parsing everything in the worker is not the
 ideal solution unless we can find a way to also keep accurate associations
 with the original nodes from content. Anything that introduces a possibly
 lossy data aspect will probably hurt translation which is already an
 innacurate science.


 Maybe you can do the translation incrementally, and just annotate the DOM
 with custom attributes (or userdata) to record the progress of the
 translation? Plus a reference to the last tranlated node (subtree) to speed
 up finding the next node subtree to translate. I assume it would be OK to
 translate one CSS block at a time.

 Rob
 --
 Jtehsauts  tshaei dS,o n Wohfy  Mdaon  yhoaus  eanuttehrotraiitny  eovni
 le atrhtohu gthot sf oirng iyvoeu rs ihnesa.rt sS?o  Whhei csha iids  teoa
 stiheer :p atroa lsyazye,d  'mYaonu,r  sGients  uapr,e  tfaokreg iyvoeunr,
 'm aotr  atnod  sgaoy ,h o'mGee.t  uTph eann dt hwea lmka'n?  gBoutt  uIp
 waanndt  wyeonut  thoo mken.o w

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

How to efficiently walk the DOM tree and its strings

[Felipe G]

Hi everyone, I'm working on a feature to offer webpage translation in
Firefox. Translation involves, quite unsurprisingly, a lot of DOM and
strings manipulation. Since DOM access only happens in the main thread, it
brings the question of how to do it properly without causing jank.

This is the use case that I'm dealing with in bug 971043:

When the user decides to translate a webpage, we want to build a tree that
is a cleaned-up version of the page's DOM tree (to remove nodes that do not
contain any useful content for translation; more details in the bug for the
curious). To do this we must visit all elements and text nodes once and
decide which ones to keep and which ones to throw away.

One idea suggested is to perform the task in chunks to let the event loop
breathe in between. The problem is that the page can dynamically change and
then a tree representation of the page may no longer exist. A possible
solution to that is to only pause the page that is being translated (with,
say, EnterModalState) until we can finish working on it, while letting
other pages and the UI work normally. That sounds a reasonable option to me
but I'd like to hear opinions.

Another option exists if it's possible to make a fast copy of the whole
DOM, and then work on this snapshot'ed copy which is not live. Better yet
if we can send this copy with a non-copy move to a Worker thread. But it
brings the question if the snapshot'ing itself won't cause jank, and if the
extra memory usage for this is worth the trade-off.

Even if we properly chunk the task, it is still bounded by the size of the
strings on the page. To decide if a text node should be kept or thrown away
we need to run a regexp on it, and there's no way to pause that midway
through. And after we have our tree representation, it must be serialized
and encodeURIComponent'ed to be sent to the translation service.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: How to efficiently walk the DOM tree and its strings

[Felipe G]

During the translation phase, Chrome imports a JS script into the webpage
and this script does all the translation work.

There's the language detection phase (another use case that I plan to ask
on a separate e-mail) in which chrome does a .textContent and run the
language detection off of it on that page's renderer thread.


On Mon, Mar 3, 2014 at 4:31 PM, Jeff Muizelaar jmuizel...@mozilla.comwrote:


 On Mar 3, 2014, at 2:28 PM, Felipe G fel...@gmail.com wrote:

  Hi everyone, I'm working on a feature to offer webpage translation in
  Firefox. Translation involves, quite unsurprisingly, a lot of DOM and
  strings manipulation. Since DOM access only happens in the main thread,
 it
  brings the question of how to do it properly without causing hank.

 What does Chrome do?

 -Jeff

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Fwd: How to efficiently walk the DOM tree and its strings

[Felipe G]

On Mon, Mar 3, 2014 at 5:19 PM, Boris Zbarsky bzbar...@mit.edu wrote:

 On 3/3/14 2:28 PM, Felipe G wrote:

 A possible solution to that is to only pause the page that is being
 translated (with,
 say, EnterModalState) until we can finish working on it, while letting
 other pages and the UI work normally.


 The other pages can still modify the DOM of the page in question, right?
  It'll just be a bit more rare...


That is true..  Although given the rarity of this case, I think I could
watch for changes and just bail out of the work if that happens.  Keeping
the page's script/events from running is probably the main thing to cover.




  Another option exists if it's possible to make a fast copy of the whole
 DOM, and then work on this snapshot'ed copy which is not live.


 How feasible is just doing .innerHTML to do that, then doing some sort of
 async parse (e.g. XHR or DOMParser) to get a DOM snapshot?  That said, this
 would mean that you end up with a snapshot that's actual DOM stuff, on the
 main thread.

 Hmm ideally I need to keep (weak) references to the text nodes from the
page in order to replace their text when translation is ready.  If I do a
.innerHTML conversion back and forth I'll lose the refs.  In theory they
can be found again but it would require (again) walking the tree and
running heuristics on it, which is potentially worse..
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform