Remote GetURIGeckoFlags | SetURIGeckoFlags

RESOLVED FIXED

Status

()

Core
IPC
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

8 years ago
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.
Will getting it from the chrome process be fast enough that it's faster than guessing wrong on the scrollbar?
(Assignee)

Comment 2

8 years ago
Created attachment 447822 [details] [diff] [review]
patch

not sure; probably something we can measure.  getting the response would involve to process switches and possible thread switches in both processes.
Assignee: nobody → dougt
Attachment #447822 - Flags: review?
(Assignee)

Updated

8 years ago
Attachment #447822 - Flags: review? → review?(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.
(Assignee)

Comment 4

8 years ago
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.
In bug 556400 we were talking about dropping nsIGlobalHistory3 all together.   Do need these bits still?
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.
> 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?
(Assignee)

Comment 8

8 years ago
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
(Assignee)

Comment 9

8 years ago
Created attachment 448021 [details] [diff] [review]
remove the calls.
Attachment #448021 - Flags: review?(roc)
(Assignee)

Updated

8 years ago
Attachment #448021 - Attachment is patch: true
Attachment #448021 - Attachment mime type: application/octet-stream → text/plain
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.
Blocks: 568971
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.
Attachment #448021 - Flags: review?(roc) → review+
fwiw, we had a bug on places for this.  It's been around for years now, obviously, with not much attention.
(Assignee)

Updated

8 years ago
Attachment #447822 - Flags: review?(bzbarsky) → review-
(Assignee)

Comment 13

8 years ago
http://hg.mozilla.org/mozilla-central/rev/c34daf577521
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Created attachment 450281 [details] [diff] [review]
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.
Attachment #450281 - Flags: review?(roc)
Version: unspecified → Trunk
Attachment #450281 - Flags: review?(roc) → review+
landed followup: http://hg.mozilla.org/mozilla-central/rev/3675017ddaa1
You need to log in before you can comment on or make changes to this bug.