Closed Bug 1448585 Opened 4 years ago Closed 4 years ago

Avoid useless use of strlen for DEFAULT_HOME_PAGE

Categories

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

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: hsivonen, Assigned: hsivonen)

References

Details

Attachments

(1 file)

Let's use AssignLiteral for a literal.
Comment on attachment 8962080 [details]
Bug 1448585 - Use NS_LITERAL_STRING for DEFAULT_HOME_PAGE. .

https://reviewboard.mozilla.org/r/230924/#review236760

::: dom/base/nsGlobalWindowOuter.cpp:4986
(Diff revision 1)
>    if (homeURL.IsEmpty()) {
>      // if all else fails, use this
>  #ifdef DEBUG_seth
>      printf("all else failed.  using %s as the home page\n", DEFAULT_HOME_PAGE);
>  #endif
> -    CopyASCIItoUTF16(DEFAULT_HOME_PAGE, homeURL);
> +    homeURL.AssignLiteral(DEFAULT_HOME_PAGE);

Let's do `homeURL = NS_LITERAL_STRING(DEFAULT_HOME_PAGE);` as that way we get to do no runtime character width changes at all.
Attachment #8962080 - Flags: review?(nika) → review+
Comment on attachment 8962080 [details]
Bug 1448585 - Use NS_LITERAL_STRING for DEFAULT_HOME_PAGE. .

https://reviewboard.mozilla.org/r/230924/#review236760

> Let's do `homeURL = NS_LITERAL_STRING(DEFAULT_HOME_PAGE);` as that way we get to do no runtime character width changes at all.

Fixed. Thanks. I wonder if we should

 1. Tell people to use `u""` literals with nsAString
 2. Have a lint to block new calls to the `char` version of `AssignLiteral` on `nsAString`
Pushed by hsivonen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6580b15bcd33
Use NS_LITERAL_STRING for DEFAULT_HOME_PAGE. r=mystor.
(In reply to Henri Sivonen (:hsivonen) from comment #4)
>  2. Have a lint to block new calls to the `char` version of `AssignLiteral`
> on `nsAString`

How about `= delete`ing the `char` overload? It would be better to find the mistake earlier than lint.
(In reply to Masatoshi Kimura [:emk] from comment #6)
> (In reply to Henri Sivonen (:hsivonen) from comment #4)
> >  2. Have a lint to block new calls to the `char` version of `AssignLiteral`
> > on `nsAString`
> 
> How about `= delete`ing the `char` overload? It would be better to find the
> mistake earlier than lint.

We could do that after changing the existing callers.
https://hg.mozilla.org/mozilla-central/rev/6580b15bcd33
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.