Closed Bug 568612 Opened 10 years ago Closed 10 years ago

Remote GetURIGeckoFlags | SetURIGeckoFlags

Categories

(Core :: IPC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dougt, Assigned: dougt)

References

Details

Attachments

(3 files)

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?
Attached patch patchSplinter Review
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?
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.
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?
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
Attachment #448021 - Flags: review?(roc)
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.
Attachment #447822 - Flags: review?(bzbarsky) → review-
http://hg.mozilla.org/mozilla-central/rev/c34daf577521
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
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.
You need to log in before you can comment on or make changes to this bug.