Make WebViewClient/WebWidgetClient parameters non-null and remove redundant null checks. |
||
Issue descriptionCurrently, it is legal to pass nullptr as WebViewClient/WebWidgetClient when creating WebView/WebWidget, but this makes the code hard to understand.
,
Mar 7 2017
Due to the object model it's probably easier to add an ASSERT() in ::create() methods. e.g. WebViewClient() has a protected dtor so we cannot take ownership in WebView.
,
Mar 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9df2c54436a76ebaf6bfcc317815e776e4a0a7f5 commit 9df2c54436a76ebaf6bfcc317815e776e4a0a7f5 Author: slangley <slangley@chromium.org> Date: Tue Mar 28 23:20:08 2017 Ensure that the WebWidgetClient cannot be a nullptr by adding DCHECK()s. There was only one call site in print_web_view_helper where a nullptr was being passed instead of a valid WebWidgetClient, to fix this the following changes were also made: * Make WebWidgetClient dtor public so the class can be instantiated. * Pass a valid WebWidgetClient in the print_web_view_helper A subsequent CL will analyze and remove m_client null checks in WebFrameWidgetImpl. I left WebFrameViewClient as a ref as this appears to be internal to blink and will adhere to blink standards. BUG= 696895 Review-Url: https://codereview.chromium.org/2773003002 Cr-Commit-Position: refs/heads/master@{#460235} [modify] https://crrev.com/9df2c54436a76ebaf6bfcc317815e776e4a0a7f5/components/printing/renderer/print_web_view_helper.cc [modify] https://crrev.com/9df2c54436a76ebaf6bfcc317815e776e4a0a7f5/content/shell/test_runner/web_view_test_client.h [modify] https://crrev.com/9df2c54436a76ebaf6bfcc317815e776e4a0a7f5/content/shell/test_runner/web_widget_test_client.h [modify] https://crrev.com/9df2c54436a76ebaf6bfcc317815e776e4a0a7f5/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp [modify] https://crrev.com/9df2c54436a76ebaf6bfcc317815e776e4a0a7f5/third_party/WebKit/Source/web/WebViewFrameWidget.cpp [modify] https://crrev.com/9df2c54436a76ebaf6bfcc317815e776e4a0a7f5/third_party/WebKit/Source/web/WebViewFrameWidget.h [modify] https://crrev.com/9df2c54436a76ebaf6bfcc317815e776e4a0a7f5/third_party/WebKit/public/web/WebViewClient.h [modify] https://crrev.com/9df2c54436a76ebaf6bfcc317815e776e4a0a7f5/third_party/WebKit/public/web/WebWidgetClient.h
,
Mar 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1a6375e628f5405f33f9061ee25644eaa90a81c2 commit 1a6375e628f5405f33f9061ee25644eaa90a81c2 Author: slangley <slangley@chromium.org> Date: Wed Mar 29 06:09:25 2017 Add canUpdateLayout & canHandleGestureEvent to WebViewClient. This work is in preperation for work to make it illegal to pass a nullptr for WebViewClient when instantiting a WebView object. There are a handful of places in WebViewImpl where the logic changes based on the nullness of WebViewClient. These changes will preserve this logic when a non-null WebViewClient is supplied. This include: - Returning WebInputEventResult::NotHandled from handleGestureEvent if the client specifically opts out of handling the event. - Not processing layout updates if the client opts out. After removing null checks in WebViewImpl the requirement for these new methods are to be reviewed and they should be removed if possible. Also, in preparation for forcing clients to pass a non null WebViewClient I made the dtor for WebViewClient public so it can be instantiated without derivation. BUG= 696895 Review-Url: https://codereview.chromium.org/2762553002 Cr-Commit-Position: refs/heads/master@{#460298} [modify] https://crrev.com/1a6375e628f5405f33f9061ee25644eaa90a81c2/components/plugins/renderer/webview_plugin.cc [modify] https://crrev.com/1a6375e628f5405f33f9061ee25644eaa90a81c2/components/plugins/renderer/webview_plugin.h [modify] https://crrev.com/1a6375e628f5405f33f9061ee25644eaa90a81c2/content/renderer/render_view_impl.cc [modify] https://crrev.com/1a6375e628f5405f33f9061ee25644eaa90a81c2/content/renderer/render_view_impl.h [modify] https://crrev.com/1a6375e628f5405f33f9061ee25644eaa90a81c2/content/shell/test_runner/web_view_test_client.cc [modify] https://crrev.com/1a6375e628f5405f33f9061ee25644eaa90a81c2/content/shell/test_runner/web_view_test_client.h [modify] https://crrev.com/1a6375e628f5405f33f9061ee25644eaa90a81c2/third_party/WebKit/Source/web/WebViewImpl.cpp [modify] https://crrev.com/1a6375e628f5405f33f9061ee25644eaa90a81c2/third_party/WebKit/Source/web/tests/FrameTestHelpers.h [modify] https://crrev.com/1a6375e628f5405f33f9061ee25644eaa90a81c2/third_party/WebKit/public/web/WebViewClient.h
,
Mar 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/943648dbbf5a074d645d71c8108c0caedff7967a commit 943648dbbf5a074d645d71c8108c0caedff7967a Author: slangley <slangley@chromium.org> Date: Thu Mar 30 01:09:10 2017 Audit use of m_client in WebFrameWidgetImpl and remove null checks. Now that a valid WebWidgetClient must always be passed when creating a WebFrameWidget we can remove some of the redundant m_client == nullptr checks. m_client can now become nullptr if close() is called on the WebFrameWidget, but this is generally caught by checking m_layerTreeView. To be safe if(m_client) checks have been replaced with DCHECK(m_client). BUG= 696895 Review-Url: https://codereview.chromium.org/2775113002 Cr-Commit-Position: refs/heads/master@{#460612} [modify] https://crrev.com/943648dbbf5a074d645d71c8108c0caedff7967a/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
,
Jan 24 2018
|
||
►
Sign in to add a comment |
||
Comment 1 by slangley@chromium.org
, Mar 1 2017