1062418 - Move the remaining nsWindowSH methods to be instance methods of nsGlobalWindow

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

This will probably mean moving a bunch of helper code too.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Move the remaining nsWindowSH bits into nsGlobalWindowInner

For now we preserve the current code structure and function signatures to make
review simplere.  That's about to get cleaned up.

MozReview-Commit-ID: 4epLHQiEwDV

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Clean up the bits that got moved from nsWindowSH

MozReview-Commit-ID: HGy6CHx4sCP

Comment 3

[Boris Zbarsky [:bzbarsky]]

> review simplere. 

"simpler".  Fixed locally.  ;)

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8956242 [details] [diff] [review]
part 2.  Clean up the bits that got moved from nsWindowSH

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

::: dom/base/nsGlobalWindowInner.cpp
@@ +16,5 @@
>  #include "nsScreen.h"
>  #include "nsHistory.h"
>  #include "nsDOMNavigationTiming.h"
>  #include "nsIDOMStorageManager.h"
> +#include "mozilla/dom/DOMJSProxyHandler.h"

I presume this is for FillPropertyDescriptor. It looks like this would've been needed in the previous patch too. I'd rather if this hunk was moved into that patch.

@@ +2938,3 @@
>  {
>    // Keep track of how often this happens.
>    Telemetry::Accumulate(Telemetry::COMPONENTS_SHIM_ACCESSED_BY_CONTENT, true);

I took a peek at this probe and it looked like ~1% of loads use this. I presume that's still high enough we can't afford to kill it yet :'-(

It'd also be good to add a comment here just explaining why this thing has to exist >.<

Comment 5

[Boris Zbarsky [:bzbarsky]]

> It looks like this would've been needed in the previous patch too.

Good catch, moved there.

> I presume that's still high enough we can't afford to kill it yet :'-(

I wonder whether it's measuring what we think it's measuring.  In particular, it's not super-obvious to me why we don't hit this for chrome windows.

> It'd also be good to add a comment here just explaining why this thing has to exist >.<

Can do.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f36074b39330
part 1.  Move the remaining nsWindowSH bits into nsGlobalWindowInner.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f2b7d7d2805
part 2.  Clean up the bits that got moved from nsWindowSH.  r=mystor

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/f36074b39330
https://hg.mozilla.org/mozilla-central/rev/2f2b7d7d2805