1479605 - Replace ENSURE_TRUE in nsFrame.h with NS_ENSURE_TRUE_VOID

Status: RESOLVED FIXED

(Core :: Layout, enhancement, P4)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

ENSURE_TRUE in nsFrame.h [1] is equivalent to NS_ENSURE_TRUE_VOID in nsDebug.h [2]. We can replace ENSURE_TRUE by NS_ENSURE_TRUE_VOID, and remove ENSURE_TRUE to avoid defining the macro twice.

[1] https://dxr.mozilla.org/mozilla-central/rev/085cdfb90903d4985f0de1dc7786522d9fb45596/layout/generic/nsFrame.h#893-901

[2] https://dxr.mozilla.org/mozilla-central/rev/085cdfb90903d4985f0de1dc7786522d9fb45596/xpcom/base/nsDebug.h#232

Comment 1

[lifanfzeng]

Can I work on this?

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Sure. Feel free to work on this. If you have any questions about building and testing Firefox, or submit a patch for review. There is some good documentation on MDN such as https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction

Comment 3

[lifanfzeng]

Sorry if this isn't the right place to ask for build help, but when I try to run the ./mach bootstrap (I'm on Windows 10, python 3.5.2 (Although the mozilla shell shows 2.7). I get the error

Error running mach:

    ['bootstrap']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

TypeError: argument of type 'NoneType' is not iterable

  File "c:\mozilla-source\mozilla-central\python/mozboot/mozboot/mach_commands.py", line 32, in bootstrap
    bootstrapper.bootstrap()
  File "c:\mozilla-source\mozilla-central\python/mozboot\mozboot\bootstrap.py", line 283, in bootstrap
    hg_installed, hg_modern = self.instance.ensure_mercurial_modern()
  File "c:\mozilla-source\mozilla-central\python/mozboot\mozboot\mozillabuild.py", line 29, in ensure_mercurial_modern
    self.run([self.which('pip'), 'install', '--upgrade', 'Mercurial'])
  File "c:\mozilla-source\mozilla-central\python/mozboot\mozboot\mozillabuild.py", line 55, in run
    subprocess.check_call(command, stdin=sys.stdin)
  File "c:\mozilla-build\python\lib\subprocess.py", line 185, in check_call
    retcode = call(*popenargs, **kwargs)
  File "c:\mozilla-build\python\lib\subprocess.py", line 172, in call
    return Popen(*popenargs, **kwargs).wait()
  File "c:\mozilla-build\python\lib\subprocess.py", line 394, in __init__
    errread, errwrite)
  File "c:\mozilla-build\python\lib\subprocess.py", line 599, in _execute_child
    args = list2cmdline(args)
  File "c:\mozilla-build\python\lib\subprocess.py", line 266, in list2cmdline
    needquote = (" " in arg) or ("\t" in arg) or not arg


The weird thing is, it used to fail on something else (The directory where my "Home" environment variable/where echo ~ would print had already been deleted to changed it to ;

What should I do? I'm thinking about reformatting my computer.

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Sorry, I'm not familiar with the Windows build environment. Perhaps you could try asking in #introduction on irc.mozilla.org. There should be some developers using Windows. https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction#Need_help

Comment 5

[Sahil Bhosale]

Can I work on this issue?

Comment 6

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

This bug has no progress so far, so anyone is welcome to contribute a patch :)

Comment 7

[Sahil Bhosale]

(In reply to Ting-Yu Lin [:TYLin] (UTC-7) from comment #0)
> ENSURE_TRUE in nsFrame.h [1] is equivalent to NS_ENSURE_TRUE_VOID in
> nsDebug.h [2]. We can replace ENSURE_TRUE by NS_ENSURE_TRUE_VOID, and remove
> ENSURE_TRUE to avoid defining the macro twice.
> 
> [1]
> https://dxr.mozilla.org/mozilla-central/rev/
> 085cdfb90903d4985f0de1dc7786522d9fb45596/layout/generic/nsFrame.h#893-901
> 
> [2]
> https://dxr.mozilla.org/mozilla-central/rev/
> 085cdfb90903d4985f0de1dc7786522d9fb45596/xpcom/base/nsDebug.h#232

When I made this changes and started to compile it gives me compilation error:

 3:23.36 In file included from /home/sahil/src/mozilla-central/obj-x86_64-pc-linux-gnu/layout/generic/Unified_cpp_layout_generic2.cpp:11:
 3:23.36 /home/sahil/src/mozilla-central/layout/generic/nsFrameSetFrame.cpp:1293:3: error: use of undeclared identifier 'ENSURE_TRUE'
 3:23.36   ENSURE_TRUE(weakFrame.IsAlive());
 3:23.37   ^
 3:24.69 1 warning generated.
 3:26.65 1 warning and 1 error generated.
 3:26.67 /home/sahil/src/mozilla-central/config/rules.mk:1088: recipe for target 'Unified_cpp_layout_generic2.o' failed
 3:26.67 make[4]: *** [Unified_cpp_layout_generic2.o] Error 1
 3:26.67 make[4]: *** Waiting for unfinished jobs....
 3:32.45 1 warning generated.
 3:38.06 1 warning generated.
 3:41.82 1 warning generated.
 3:41.86 /home/sahil/src/mozilla-central/config/recurse.mk:74: recipe for target 'layout/generic/target' failed
 3:41.86 make[3]: *** [layout/generic/target] Error 2
 3:47.18 1 warning generated.
 3:47.22 /home/sahil/src/mozilla-central/config/recurse.mk:32: recipe for target 'compile' failed
 3:47.22 make[2]: *** [compile] Error 2
 3:47.22 /home/sahil/src/mozilla-central/config/rules.mk:423: recipe for target 'default' failed
 3:47.22 make[1]: *** [default] Error 2
 3:47.22 client.mk:150: recipe for target 'build' failed
 3:47.22 make: *** [build] Error 2
 3:47.25 254 compiler warnings present.

Comment 8

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

(In reply to Sahil Bhosale from comment #7)
 
>  3:23.36 In file included from
> /home/sahil/src/mozilla-central/obj-x86_64-pc-linux-gnu/layout/generic/
> Unified_cpp_layout_generic2.cpp:11:
>  3:23.36
> /home/sahil/src/mozilla-central/layout/generic/nsFrameSetFrame.cpp:1293:3:
> error: use of undeclared identifier 'ENSURE_TRUE'
>  3:23.36   ENSURE_TRUE(weakFrame.IsAlive());

Looks like you forget to change ENSURE_TRUE to NS_ENSURE_TRUE_VOID?

Comment 9

[Sahil Bhosale]

Attachment: tip2.patch

I have changed the ENSURE_TRUE to NS_ENSURE_TRUE_VOID.

Comment 10

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8999447 [details] [diff] [review]
tip2.patch

Review of attachment 8999447 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me. I'll accept this patch if you address my other inline comment.

::: layout/generic/nsFrame.h
@@ +897,3 @@
>  // End Display Reflow Debugging
>  
>  // similar to NS_ENSURE_TRUE but with no return value

No other code uses ENSURE_TRUE() macro after your patch, so please delete the comment as well as this macro definition.

Comment 11

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Also, your patch lacks a proper commit message to be able to check in. You can use something like

   Bug 1479605 - Replace ENSURE_TRUE in nsFrame.h by NS_ENSURE_TRUE_VOID.

If you're using hg, commit with your changes and the message, and use "hg export . > my-change.patch" to make sure the patch is generated with the commit message. If you're using git, it's "git format-patch @~".

Comment 12

[Sahil Bhosale]

Attachment: tip.patch

Replace ENSURE_TRUE in nsFrame.h by NS_ENSURE_TRUE_VOID.

Comment 13

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8999480 [details] [diff] [review]
tip.patch

Review of attachment 8999480 [details] [diff] [review]:
-----------------------------------------------------------------

I still don't see the commit message in this patch. If you have any problem with the step in comment 11, please let me know. Or I can help adding a commit message for you before landing the patch if you'd like. Thanks.

::: layout/generic/nsFrame.h
@@ -898,2 @@
>  
> -// similar to NS_ENSURE_TRUE but with no return value

Err, sorry to be unclear in comment 10. I mean delete the original line 899 - 906, i.e. the following.

  899 // similar to NS_ENSURE_TRUE ...
      ...
  906 PR_END_MACRO

NS_ENSURE_TRUE_VOID is already defined in xpcom/base/nsDebug.h, so no need to redefine it here.

Comment 14

[Sahil Bhosale]

Yes I am getting a problem while commit as when create my patch using hg diff >tip patch and hg export -o FILE then everything is fine but after that if I run hg commit then vi editor gets open and when I type the commit message and save then after that the contents in my patch file disappears and my tip.patch file becomes blank. 

From yesterday only I am facing this problem can you please help? 

Even I tried to delete the source code and re-download and compiled it and made changes and created the patch and after running hg commit the same thing happen i.e my tip.patch file became blank and all my contents in the patch file lost.

Comment 15

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Hmm ... I cannot see the problem from your description. In theory, after you commit with "hg commit", and you are able to see your modification by "hg log -p -r .", then "hg export -r . -o my.patch" should generate the same patch content.

Anyway, don't worry about the commit message too much, I can fix the message for your in your new patch after you address my request in comment 13.

Comment 16

[Sahil Bhosale]

Attachment: final.patch

The changes which you have suggested I have made those changes. Can you please add the commit message for this bug because I tried it but was unable to achieve it.

Comment 17

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8999515 [details] [diff] [review]
final.patch

Review of attachment 8999515 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me. Thank you for working on this. I'll upload a landable patch with commit message later.

Comment 18

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Remove ENSURE_TRUE in nsFrame.h, and replace it with NS_ENSURE_TRUE_VOID.

Comment 19

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Remove ENSURE_TRUE in nsFrame.h, and replace it with NS_ENSURE_TRUE_VOID.

Comment 20

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4f754ae0369e2c1e44d18231da5bf3e114124578

Comment 21

[Sahil Bhosale]

(In reply to Ting-Yu Lin [:TYLin] (UTC-7) from comment #19)
> Created attachment 8999723 [details] [diff] [review]
> Remove ENSURE_TRUE in nsFrame.h, and replace it with NS_ENSURE_TRUE_VOID.

Thanks Ting-Yu Lin for adding the commit message for me. 

I have a question, I want to submit the patches through command line but when I run this command "hg push review" it is giving me an error which says: abort access denied (public key) . 
I have also added the API key then why I am getting this error can you please help me?

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a627240fa20
Remove ENSURE_TRUE in nsFrame.h, and replace it with NS_ENSURE_TRUE_VOID. r=TYLin

Comment 23

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/9a627240fa20

Comment 24

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

(In reply to Sahil Bhosale from comment #21)
> (In reply to Ting-Yu Lin [:TYLin] (UTC-7) from comment #19)
> > Created attachment 8999723 [details] [diff] [review]
> > Remove ENSURE_TRUE in nsFrame.h, and replace it with NS_ENSURE_TRUE_VOID.
> 
> Thanks Ting-Yu Lin for adding the commit message for me. 

You're welcome!

> I have a question, I want to submit the patches through command line but
> when I run this command "hg push review" it is giving me an error which
> says: abort access denied (public key) . 
> I have also added the API key then why I am getting this error can you
> please help me?

"hg push review" will push to mozreview, which is decommissioned in early August. See [1] for the announcement if you're interested.

If you'd like to submit patches for other bugs, you can still use attachments to bugzilla as you did. You can also setup Phabricator [2], which is the replacement for mozreview.

[1] https://groups.google.com/d/msg/mozilla.dev.platform/Bxomd5VAgKU/8Z3o5F5kCAAJ
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction#Step_4_Get_your_code_reviewed

Comment 25

[Sahil Bhosale]

 > "hg push review" will push to mozreview, which is decommissioned in early
> August. See [1] for the announcement if you're interested.
> 
> If you'd like to submit patches for other bugs, you can still use
> attachments to bugzilla as you did. You can also setup Phabricator [2],
> which is the replacement for mozreview.
> 
> [1]
> https://groups.google.com/d/msg/mozilla.dev.platform/Bxomd5VAgKU/8Z3o5F5kCAAJ
> [2]
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> Introduction#Step_4_Get_your_code_reviewed


Thanks for sharing this post.