nsIWebNavigation::LOAD_FLAGS_BYPASS_CLASSIFIER load flag is greater than nsIWebNavigation::LOAD_FLAGS_MASK

RESOLVED INVALID

Status

()

Core
Networking
RESOLVED INVALID
10 years ago
2 years ago

People

(Reporter: Gavin, Assigned: Jeffrey Tran)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [necko-backlog][good first bug])

Attachments

(1 attachment)

Bug 413938 added LOAD_FLAGS_BYPASS_CLASSIFIER = 0x10000, which is greater than LOAD_FLAGS_MASK (0xffff).

We don't use LOAD_FLAGS_MASK in Mozilla code, and biesi says he isn't sure why it's useful, so perhaps we should just remove it.
Whiteboard: [necko-backlog][good first bug]
(Assignee)

Comment 1

2 years ago
Created attachment 8718212 [details] [diff] [review]
rev1 - Remove LOAD_FLAGS_MASK from nsIWebNavigation.idl

Removed LOAD_FLAGS_MASK from nsIWebNavigation.idl
Attachment #8718212 - Flags: review?(gavin.sharp)
(Assignee)

Updated

2 years ago
Assignee: nobody → tranj23
Attachment #8718212 - Flags: review?(gavin.sharp) → review?(bzbarsky)
Comment on attachment 8718212 [details] [diff] [review]
rev1 - Remove LOAD_FLAGS_MASK from nsIWebNavigation.idl

What exactly is the point of this patch?  The comment is correct: flags not inside LOAD_FLAGS_MASK can't be passed to Reload().  Reload() doesn't use LOAD_FLAGS_MASK, but it does assert that the flags it's given don't involve EXTRA_LOAD_FLAGS which is a bitfield that includes 0xffff0000.  I suppose we could rewrite that as ~LOAD_FLAGS_MASK if we really wanted to....
Attachment #8718212 - Flags: review?(bzbarsky) → review-
Oh, and this constant is "useful" because the load flags end up being shifted up by 16 bits by MAKE_LOAD_TYPE, so any flags outside LOAD_FLAGS_MASK that make it to the MAKE_LOAD_TYPE callsite will be lost.  Which means that any time you add a load flag outside this range you have to very carefully handle it before we MAKE_LOAD_TYPE.

This bug, as filed, seems to be either invalid or worksforme to me, since we did add just such hacky support for LOAD_FLAGS_BYPASS_CLASSIFIER.  Gavin?
Flags: needinfo?(gavin.sharp)
Perhaps it could be morphed into "the whole load flag setup is horribly confusing", but that's probably not an actionable bug.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Flags: needinfo?(gavin.sharp)
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.