1448585 - Avoid useless use of strlen for DEFAULT_HOME_PAGE

Status: RESOLVED FIXED

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

Description

[Henri Sivonen (:hsivonen)]

Let's use AssignLiteral for a literal.

Comment 1

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1448585 - Use NS_LITERAL_STRING for DEFAULT_HOME_PAGE. .

Review commit: https://reviewboard.mozilla.org/r/230924/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/230924/

Comment 2

[Nika Layzell [:nika] (ni? for response)]

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.

Comment 3

[Henri Sivonen (:hsivonen)]

Comment on attachment 8962080 [details]
Bug 1448585 - Use NS_LITERAL_STRING for DEFAULT_HOME_PAGE. .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/230924/diff/1-2/

Comment 4

[Henri Sivonen (:hsivonen)]

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`

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6580b15bcd33
Use NS_LITERAL_STRING for DEFAULT_HOME_PAGE. r=mystor.

Comment 6

[Masatoshi Kimura [:emk]]

(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.

Comment 7

[Henri Sivonen (:hsivonen)]

(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.

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/6580b15bcd33