Bug 119165 – REGRESSION(r153380): Can't open messages on Gmail

• Component: New Bugs
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Reproduction steps:
1. Login to Gmail
2. Click on any message.

Expected result:
The message is open on a pane

Actual result:
Nothing happens.

I don't see any error messages in the console either.  It appears as if click events are never fired?

[Radar WebKit Bug Importer]

<rdar://problem/14564599>

[Filip Pizlo]

It worked fine when I was landing http://trac.webkit.org/changeset/153381; after all, to get the crash to repro you had to click on messages.

[Ryosuke Niwa]

I guess the regression range is somewhat misleading since WebKit nightly used to crash prior to http://trac.webkit.org/changeset/153381

[Radar WebKit Bug Importer]

<rdar://problem/14564630>

[Radar WebKit Bug Importer]

<rdar://problem/14564632>

[Ryosuke Niwa]

(In reply to comment #2)
> It worked fine when I was landing http://trac.webkit.org/changeset/153381; after all, to get the crash to repro you had to click on messages.

Do you remember the revision you were able to open a message?

[Ryosuke Niwa]

This is probably caused by http://trac.webkit.org/changeset/153380.

[Ryosuke Niwa]

I've confirmed that the regression is caused by http://trac.webkit.org/changeset/153380 by manually bisecting the issue.

[Ryosuke Niwa]

kling points out that the issue doesn't reproduce on this local machine. We're suspecting it's yet another instance of an old version of clang not being able to compile FINAL correctly.

[Ryosuke Niwa]

It appears that this bug reproduces whenever we compile WebKit with clang 4 but not when we compile it with clang 5.

[Ryosuke Niwa]

Created attachment 207569 [details]
Disables final on clang 4.0

[Ryosuke Niwa]

Created attachment 207570 [details]
Fixed a typo

[Ryosuke Niwa]

Comment on attachment 207570 [details]
Fixed a typo

View in context: https://bugs.webkit.org/attachment.cgi?id=207570&action=review

> Source/WTF/wtf/Compiler.h:44
> +#define GCC_VERSION_AT_LEAST(major, minor) (__clang_major__ >= major && __clang_major__ >= minor)

Ugh... clearly this needs to be CLANG_VERSION_AT_LEAST instead.

> Source/WTF/wtf/Compiler.h:65
> +#define WTF_COMPILER_SUPPORTS_CXX_FINAL_CONTROL WTF_COMPILER_SUPPORTS_CXX_OVERRIDE_CONTROL && GCC_VERSION_AT_LEAST(4, 2)

Ditto here.

[Ryosuke Niwa]

Created attachment 207571 [details]
Patch for landing

[Ryosuke Niwa]

Comment on attachment 207571 [details]
Patch for landing

I'm gonna wait for EWS bots to process this patch.

[Andreas Kling]

Comment on attachment 207571 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=207571&action=review

> Source/WTF/wtf/Compiler.h:44
> +#define CLANG_VERSION_AT_LEAST(major, minor) (defined(__clang_major__) && __clang_major__ >= major && __clang_major__ >= minor)

Typo at the end, __clang_major__ >= minor
Should be __clang_minor__ >= minor

[Ryosuke Niwa]

Created attachment 207574 [details]
Make the fix Apple specific since __clang_major__ and __clang_minor__ are vendor dependent

[WebKit Commit Bot]

Attachment 207574 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Compiler.h']" exit_code: 1
Source/WTF/wtf/Compiler.h:63:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/Compiler.h:63:  Should have a space between // and comment  [whitespace/comments] [4]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ryosuke Niwa]

Created attachment 207575 [details]
Use ifdef instead of if defined

[WebKit Commit Bot]

Attachment 207575 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Compiler.h']" exit_code: 1
Source/WTF/wtf/Compiler.h:63:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/Compiler.h:63:  Should have a space between // and comment  [whitespace/comments] [4]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andreas Kling]

Comment on attachment 207575 [details]
Use ifdef instead of if defined

Looks like a reasonable solution. I'll land this patch right away to get nightlies back in shape on ML.

[Andreas Kling]

Comment on attachment 207575 [details]
Use ifdef instead of if defined

Clearing flags on attachment: 207575

Committed r153400: <http://trac.webkit.org/changeset/153400>

[Andreas Kling]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

(In reply to comment #21)
> (From update of attachment 207575 [details])
> Looks like a reasonable solution. I'll land this patch right away to get nightlies back in shape on ML.

Thanks!

[Alexey Proskuryakov]

*** Bug 119176 has been marked as a duplicate of this bug. ***

[Alexey Proskuryakov]

*** Bug 119193 has been marked as a duplicate of this bug. ***

[Alexey Proskuryakov]

*** Bug 119202 has been marked as a duplicate of this bug. ***