Add thread violation checks to WebView public APIs.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 13 Feb 2016 00:22:53 +0000 (00:22 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 13 Feb 2016 00:22:53 +0000 (00:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154183

Reviewed by Geoffrey Garen.

This will help clients of the API detect the violations sooner rather than
having to debug mysterious crashes / failures later.

To that end, I've added thread violation checks to the following functions
because ...

* WebView/WebView.mm:
(-[WebView setCustomTextEncodingName:]):
- Uses the FrameLoader (which is for the main thread only).

(-[WebView stringByEvaluatingJavaScriptFromString:]):
- Invokes JavaScript (which is for the main thread only).

(-[WebView windowScriptObject]):
- Invokes ScriptController::windowScriptObject() which requires the JSLock.

(-[WebView setGroupName:]):
- Manipulates the PageGroup and Page (which is for the main thread only).

(-[WebView setMainFrameURL:]):
- Uses the FrameLoader (which is for the main thread only).

(-[WebView mainFrameTitle]):
- Uses the FrameLoader::documentLoader() (via [WebFrame _dataSource]) which
  is RefPtr, and therefore not safe for other threads to access.

(-[WebView mainFrameIcon]):
- Uses the FrameLoader::documentLoader() (via [WebFrame _dataSource]) which
  is RefPtr, and therefore not safe for other threads to access.
- Uses [WebIconDatabase sharedIconDatabase] which does a singleton
  instantiation but is not protected by a lock.

(-[WebView setDrawsBackground:]):
- Potentially manipulates a RenderView (via FrameView::setBaseBackgroundColor,
  via [WebFrame _updateBackgroundAndUpdatesWhileOffscreen]), and RenderView
  is for main thread only use.

(-[WebView setShouldUpdateWhileOffscreen:]):
- Uses [WebFrame _updateBackgroundAndUpdatesWhileOffscreen].  Hence, for the
  main thread only.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@196527 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebView.mm

index 490c084..9a47ba7 100644 (file)
@@ -1,3 +1,51 @@
+2016-02-12  Mark Lam  <mark.lam@apple.com>
+
+        Add thread violation checks to WebView public APIs.
+        https://bugs.webkit.org/show_bug.cgi?id=154183
+
+        Reviewed by Geoffrey Garen.
+
+        This will help clients of the API detect the violations sooner rather than
+        having to debug mysterious crashes / failures later.
+
+        To that end, I've added thread violation checks to the following functions
+        because ...
+
+        * WebView/WebView.mm:
+        (-[WebView setCustomTextEncodingName:]):
+        - Uses the FrameLoader (which is for the main thread only).
+
+        (-[WebView stringByEvaluatingJavaScriptFromString:]):
+        - Invokes JavaScript (which is for the main thread only).
+
+        (-[WebView windowScriptObject]):
+        - Invokes ScriptController::windowScriptObject() which requires the JSLock.
+
+        (-[WebView setGroupName:]):
+        - Manipulates the PageGroup and Page (which is for the main thread only).
+
+        (-[WebView setMainFrameURL:]):
+        - Uses the FrameLoader (which is for the main thread only).
+
+        (-[WebView mainFrameTitle]):
+        - Uses the FrameLoader::documentLoader() (via [WebFrame _dataSource]) which
+          is RefPtr, and therefore not safe for other threads to access.
+
+        (-[WebView mainFrameIcon]):
+        - Uses the FrameLoader::documentLoader() (via [WebFrame _dataSource]) which
+          is RefPtr, and therefore not safe for other threads to access.
+        - Uses [WebIconDatabase sharedIconDatabase] which does a singleton
+          instantiation but is not protected by a lock.
+
+        (-[WebView setDrawsBackground:]):
+        - Potentially manipulates a RenderView (via FrameView::setBaseBackgroundColor,
+          via [WebFrame _updateBackgroundAndUpdatesWhileOffscreen]), and RenderView
+          is for main thread only use.
+
+        (-[WebView setShouldUpdateWhileOffscreen:]):
+        - Uses [WebFrame _updateBackgroundAndUpdatesWhileOffscreen].  Hence, for the
+          main thread only.
+
 2016-02-12  Sukolsak Sakshuwong  <sukolsak@gmail.com>
 
         Update ICU header files to version 52
index 5402363..25450be 100644 (file)
@@ -5843,6 +5843,8 @@ static NSString * const backingPropertyOldScaleFactorKey = @"NSBackingPropertyOl
 
 - (void)setCustomTextEncodingName:(NSString *)encoding
 {
+    WebCoreThreadViolationCheckRoundTwo();
+
     NSString *oldEncoding = [self customTextEncodingName];
     if (encoding == oldEncoding || [encoding isEqualToString:oldEncoding])
         return;
@@ -5867,6 +5869,8 @@ static NSString * const backingPropertyOldScaleFactorKey = @"NSBackingPropertyOl
 
 - (NSString *)stringByEvaluatingJavaScriptFromString:(NSString *)script
 {
+    WebCoreThreadViolationCheckRoundTwo();
+
 #if !PLATFORM(IOS)
     // Return statements are only valid in a function but some applications pass in scripts
     // prefixed with return (<rdar://problems/5103720&4616860>) since older WebKit versions
@@ -5889,6 +5893,8 @@ static NSString * const backingPropertyOldScaleFactorKey = @"NSBackingPropertyOl
 
 - (WebScriptObject *)windowScriptObject
 {
+    WebCoreThreadViolationCheckRoundTwo();
+
     Frame* coreFrame = [self _mainCoreFrame];
     if (!coreFrame)
         return nil;
@@ -6154,6 +6160,8 @@ static WebFrame *incrementFrame(WebFrame *frame, WebFindOptions options = 0)
 
 - (void)setGroupName:(NSString *)groupName
 {
+    WebCoreThreadViolationCheckRoundTwo();
+
     if (_private->group)
         _private->group->removeWebView(self);
 
@@ -6244,6 +6252,8 @@ static WebFrame *incrementFrame(WebFrame *frame, WebFindOptions options = 0)
 
 - (void)setMainFrameURL:(NSString *)URLString
 {
+    WebCoreThreadViolationCheckRoundTwo();
+
     NSURL *url;
     if ([URLString hasPrefix:@"/"])
         url = [NSURL fileURLWithPath:URLString];
@@ -6270,6 +6280,8 @@ static WebFrame *incrementFrame(WebFrame *frame, WebFindOptions options = 0)
 
 - (NSString *)mainFrameTitle
 {
+    WebCoreThreadViolationCheckRoundTwo();
+
     NSString *mainFrameTitle = [[[self mainFrame] _dataSource] pageTitle];
     return (mainFrameTitle != nil) ? mainFrameTitle : (NSString *)@"";
 }
@@ -6277,6 +6289,8 @@ static WebFrame *incrementFrame(WebFrame *frame, WebFindOptions options = 0)
 #if !PLATFORM(IOS)
 - (NSImage *)mainFrameIcon
 {
+    WebCoreThreadViolationCheckRoundTwo();
+
     return [[WebIconDatabase sharedIconDatabase] iconForURL:[[[[self mainFrame] _dataSource] _URL] _web_originalDataAsString] withSize:WebIconSmallSize];
 }
 #else
@@ -6303,6 +6317,8 @@ static WebFrame *incrementFrame(WebFrame *frame, WebFindOptions options = 0)
 
 - (void)setDrawsBackground:(BOOL)drawsBackground
 {
+    WebCoreThreadViolationCheckRoundTwo();
+
     if (_private->drawsBackground == drawsBackground)
         return;
     _private->drawsBackground = drawsBackground;
@@ -6318,6 +6334,8 @@ static WebFrame *incrementFrame(WebFrame *frame, WebFindOptions options = 0)
 
 - (void)setShouldUpdateWhileOffscreen:(BOOL)updateWhileOffscreen
 {
+    WebCoreThreadViolationCheckRoundTwo();
+
     if (_private->shouldUpdateWhileOffscreen == updateWhileOffscreen)
         return;
     _private->shouldUpdateWhileOffscreen = updateWhileOffscreen;