568612 - Remote GetURIGeckoFlags | SetURIGeckoFlags

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Doug Turner (:dougt)]

http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsIGlobalHistory3.idl#90

Currently we use this api to remember if a scrollbar is needed or not for a given uri.


I suppose it needs to be remoted.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Will getting it from the chrome process be fast enough that it's faster than guessing wrong on the scrollbar?

Comment 2

[Doug Turner (:dougt)]

Attachment: patch

not sure; probably something we can measure.  getting the response would involve to process switches and possible thread switches in both processes.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Well, hold on.  In the chrome process there's no point for this.  This is only worth it in the process that handles web content.

Comment 4

[Doug Turner (:dougt)]

bz, consider the case where there is only one process (e.g. the chrome process).  This is the case that Firefox desktop will be in until it supports e10s.  It will have MOZ_IPC defined, but no elements will be remoted.  Surely, in this case, we should attempt to remember the gecko flags.

Comment 5

[Shawn Wilsher :sdwilsh]

In bug 556400 we were talking about dropping nsIGlobalHistory3 all together.   Do need these bits still?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

When I landed this, the speedup we got was very small.

It's probably worth re-running Tp4 to see if there's measurable difference with them removed.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> Surely, in this case, we should attempt to remember the gecko flags.

Should we?  If we're planning to stop doing this anyway, why not stop now and remove complexity instead of adding it?

Comment 8

[Doug Turner (:dougt)]

roc - the reason that it didn't make any speed up was that the history backed didn't record anything:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/src/nsNavHistory.cpp#5535

Comment 9

[Doug Turner (:dougt)]

Attachment: remove the calls.

Comment 10

[Boris Zbarsky [:bzbarsky]]

fwiw, when roc added this code (the time he's referring to in comment 6) is pre-places; at the time the history backend did in fact record the information.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 448021 [details] [diff] [review]
remove the calls.

I'm not completely happy that that optimization was just disabled by Places, but I guess this is the best thing to do for now.

Comment 12

[Boris Zbarsky [:bzbarsky]]

fwiw, we had a bug on places for this.  It's been around for years now, obviously, with not much attention.

Comment 13

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/c34daf577521

Comment 14

[Daniel Holbert [:dholbert]]

Attachment: followup: remove GetDocURI()

This bug's patch removed the last use of the static helper-function "GetDocURI", so now we get:
> layout/generic/nsGfxScrollFrame.cpp:3302: warning: 'nsIURI* GetDocURI(nsIFrame*)' defined but not used

The attached followup-patch removes this method.

Comment 15

[Daniel Holbert [:dholbert]]

landed followup: http://hg.mozilla.org/mozilla-central/rev/3675017ddaa1