Guess what this patch will be about
Created attachment 316003 [details] Patch
Attachment 316003 [details] did not pass style-queue: ERROR: Source/WebKit/WebProcess/Plugins/PluginView.cpp:944: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 1 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 316003 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316003&action=review > Source/WebCore/dom/EventTarget.cpp:236 > + auto& context = *scriptExecutionContext(); Is scriptExectutionContext() guaranteed to never return nullptr? Either it should be changed to return a ref, or there should be a null check, or there should be an assert that it is not null.
(In reply to Jeremy Jones from comment #3) > Comment on attachment 316003 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316003&action=review > > > Source/WebCore/dom/EventTarget.cpp:236 > > + auto& context = *scriptExecutionContext(); > > Is scriptExectutionContext() guaranteed to never return nullptr? > Either it should be changed to return a ref, or there should be a null > check, or there should be an assert that it is not null. In this case it's guaranteed (event listeners shouldn't be firing outside of a script execution context) and we were already dereferencing the pointer a few lines below. But you are right, let's add an assertion for this!
Created attachment 316015 [details] Patch II Added assertion per Jeremy's comment. Try to fix up Windows and Gtk+ builds.
Attachment 316015 [details] did not pass style-queue: WARNING: File exempt from style guide. Skipping: "Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/GObjectEventListener.cpp" WARNING: File exempt from style guide. Skipping: "Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/GObjectEventListener.h" ERROR: Source/WebKit/WebProcess/Plugins/PluginView.cpp:944: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebKitLegacy/win/DOMEventsClasses.cpp:79: 'ePtr' is incorrectly named. It should be named 'protector' or 'protectedE'. [readability/naming/protected] [4] Total errors found: 2 in 42 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 316015 [details] Patch II r=me Is Windows broken?
(In reply to Andreas Kling from comment #4) > (In reply to Jeremy Jones from comment #3) > > Comment on attachment 316003 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=316003&action=review > > > > > Source/WebCore/dom/EventTarget.cpp:236 > > > + auto& context = *scriptExecutionContext(); > > > > Is scriptExectutionContext() guaranteed to never return nullptr? > > Either it should be changed to return a ref, or there should be a null > > check, or there should be an assert that it is not null. > > In this case it's guaranteed (event listeners shouldn't be firing outside of > a script execution context) and we were already dereferencing the pointer a > few lines below. > > But you are right, let's add an assertion for this! provisional r+
Created attachment 316104 [details] Patch for landing
Attachment 316104 [details] did not pass style-queue: WARNING: File exempt from style guide. Skipping: "Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/GObjectEventListener.cpp" ERROR: Source/WebKitLegacy/win/Plugins/PluginViewWin.cpp:626: Multi line control clauses should use braces. [whitespace/braces] [4] WARNING: File exempt from style guide. Skipping: "Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/GObjectEventListener.h" ERROR: Source/WebKit/WebProcess/Plugins/PluginView.cpp:944: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebKitLegacy/win/DOMEventsClasses.cpp:79: 'ePtr' is incorrectly named. It should be named 'protector' or 'protectedE'. [readability/naming/protected] [4] Total errors found: 3 in 43 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 316104 [details] Patch for landing Clearing flags on attachment: 316104 Committed r219743: <http://trac.webkit.org/changeset/219743>
All reviewed patches have been landed. Closing bug.