Bug 111949 - Not for landing, bot test only
Summary: Not for landing, bot test only
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-10 17:12 PDT by Kassy Coan
Modified: 2013-09-12 22:52 PDT (History)
12 users (show)

See Also:


Attachments
Patch (8.62 KB, patch)
2013-03-10 17:13 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Patch (8.62 KB, patch)
2013-03-18 16:14 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Patch (8.64 KB, patch)
2013-03-18 18:27 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Patch (8.84 KB, patch)
2013-03-19 02:55 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Patch (33.72 KB, patch)
2013-03-19 18:18 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Patch (35.82 KB, patch)
2013-03-19 22:17 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Patch (29.87 KB, patch)
2013-03-21 00:21 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Patch (30.00 KB, patch)
2013-03-21 16:32 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-16 (8.33 MB, application/zip)
2013-03-21 17:40 PDT, Build Bot
no flags Details
Patch (29.96 KB, patch)
2013-03-21 17:50 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Switch implementation (30.50 KB, patch)
2013-03-21 21:53 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-14 (577.05 KB, application/zip)
2013-03-21 22:41 PDT, Build Bot
no flags Details
Patch (30.50 KB, patch)
2013-03-22 04:28 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Patch (29.10 KB, patch)
2013-03-22 06:00 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 (707.44 KB, application/zip)
2013-03-22 08:11 PDT, Build Bot
no flags Details
Patch (29.85 KB, patch)
2013-03-22 17:53 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Patch (30.43 KB, patch)
2013-03-22 18:04 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-08 for chromium-linux-x86_64 (669.18 KB, application/zip)
2013-03-22 20:29 PDT, WebKit Review Bot
no flags Details
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 (521.69 KB, application/zip)
2013-03-23 02:35 PDT, Build Bot
no flags Details
Document as RefPtr (29.38 KB, patch)
2013-03-24 17:37 PDT, Kassy Coan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kassy Coan 2013-03-10 17:12:23 PDT
Not for landing, bot test only
Comment 1 Kassy Coan 2013-03-10 17:13:23 PDT
Created attachment 192393 [details]
Patch
Comment 2 Build Bot 2013-03-10 18:48:19 PDT
Comment on attachment 192393 [details]
Patch

Attachment 192393 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17131121

New failing tests:
inspector/extensions/extensions-resources.html
inspector/styles/get-set-stylesheet-text.html
Comment 3 Build Bot 2013-03-11 09:39:51 PDT
Comment on attachment 192393 [details]
Patch

Attachment 192393 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17161216

New failing tests:
inspector/extensions/extensions-resources.html
Comment 4 Kassy Coan 2013-03-18 16:14:21 PDT
Created attachment 193680 [details]
Patch
Comment 5 Kassy Coan 2013-03-18 18:27:46 PDT
Created attachment 193710 [details]
Patch
Comment 6 Build Bot 2013-03-19 00:56:31 PDT
Comment on attachment 193710 [details]
Patch

Attachment 193710 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17213547

New failing tests:
inspector/styles/get-set-stylesheet-text.html
Comment 7 Kassy Coan 2013-03-19 02:55:06 PDT
Created attachment 193771 [details]
Patch
Comment 8 Build Bot 2013-03-19 12:17:43 PDT
Comment on attachment 193771 [details]
Patch

Attachment 193771 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17222558

New failing tests:
inspector/extensions/extensions-resources.html
Comment 9 Kassy Coan 2013-03-19 18:18:16 PDT
Created attachment 193962 [details]
Patch
Comment 10 Early Warning System Bot 2013-03-19 18:31:05 PDT
Comment on attachment 193962 [details]
Patch

Attachment 193962 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17215719
Comment 11 Build Bot 2013-03-19 18:32:10 PDT
Comment on attachment 193962 [details]
Patch

Attachment 193962 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17215717
Comment 12 EFL EWS Bot 2013-03-19 18:33:59 PDT
Comment on attachment 193962 [details]
Patch

Attachment 193962 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17129699
Comment 13 Early Warning System Bot 2013-03-19 18:35:30 PDT
Comment on attachment 193962 [details]
Patch

Attachment 193962 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17129700
Comment 14 Build Bot 2013-03-19 18:51:13 PDT
Comment on attachment 193962 [details]
Patch

Attachment 193962 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17125683
Comment 15 Kassy Coan 2013-03-19 22:17:33 PDT
Created attachment 193984 [details]
Patch
Comment 16 Kassy Coan 2013-03-21 00:21:37 PDT
Created attachment 194196 [details]
Patch
Comment 17 Build Bot 2013-03-21 01:04:35 PDT
Comment on attachment 194196 [details]
Patch

Attachment 194196 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17226354
Comment 18 Build Bot 2013-03-21 04:12:29 PDT
Comment on attachment 194196 [details]
Patch

Attachment 194196 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17193533
Comment 19 Kassy Coan 2013-03-21 16:32:32 PDT
Created attachment 194378 [details]
Patch
Comment 20 Build Bot 2013-03-21 17:40:54 PDT
Comment on attachment 194378 [details]
Patch

Attachment 194378 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17213809

New failing tests:
inspector/extensions/extensions-resources.html
inspector/styles/get-set-stylesheet-text.html
Comment 21 Build Bot 2013-03-21 17:40:56 PDT
Created attachment 194398 [details]
Archive of layout-test-results from webkit-ews-16

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: <class 'webkitpy.common.config.ports.MacWK2Port'>  Platform: Mac OS X 10.8.2
Comment 22 Kassy Coan 2013-03-21 17:50:10 PDT
Created attachment 194402 [details]
Patch
Comment 23 Mike Lawther 2013-03-21 19:10:55 PDT
Comment on attachment 194402 [details]
Patch

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

> Source/WebCore/page/FeatureObserver.cpp:50
> +    map.add(CSSPropertyColor, 2);

tbh, I was expecting this to be a large switch statement, eg

enum HistogramId {
  HistogramIdColor = 2,
  HistogramIdDirection = 3,
  HistogramIdDisplay = 4,
 ....
  HistogramIdMaximum
  // add new values above the maximum
};

static int getHistogramId(CSSPropertyId id) {
  switch(id) {
  case CSSPropertyIdColor:
    return HistogramIdColor;
  case CSSPropertyIdDirection:
    return HistogramIdDirection;
 ....
   default:
    ASSERT_NOT_REACHED();
  }  

  return -1;  
}

Extraordinarily boring like I said. 

How come you went with the HashMap?
Comment 24 Kassy Coan 2013-03-21 21:46:51 PDT
I ran profiling for both methods (count is the number of properties at that histogram flush):

Elapsed Time MAP: 0.000120 count: 105
Elapsed Time SWITCH: 0.000080 count: 105

Elapsed Time MAP: 0.000001 count: 0
Elapsed Time SWITCH: 0.000000 count: 0

Elapsed Time MAP: 0.000039 count: 103
Elapsed Time SWITCH: 0.000031 count: 103

Elapsed Time MAP: 0.000035 count: 95
Elapsed Time SWITCH: 0.000029 count: 95

Elapsed Time MAP: 0.000035 count: 95
Elapsed Time SWITCH: 0.000031 count: 95

Elapsed Time MAP: 0.000035 count: 94
Elapsed Time SWITCH: 0.000028 count: 94

I had gone with the map because I thought it would be faster, and it seemed like good practice. However, it looks like the switch is consistently marginally faster.

I'm uploading the switch implementation for bot testing now.
(The previous implementation was passing on the local mac machines, so I expect this to have no trouble).

This switch implementation assigns the constants directly, without use of the enumeration. I did this for 2 reasons.
1. The code reads clearly without use of the enum
2. It makes updating/adding in new properties easier with fewer steps.
Comment 25 Kassy Coan 2013-03-21 21:53:46 PDT
Created attachment 194440 [details]
Switch implementation
Comment 26 Build Bot 2013-03-21 22:41:42 PDT
Comment on attachment 194440 [details]
Switch implementation

Attachment 194440 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17191727

New failing tests:
inspector/extensions/extensions-resources.html
inspector/styles/get-set-stylesheet-text.html
Comment 27 Build Bot 2013-03-21 22:41:44 PDT
Created attachment 194445 [details]
Archive of layout-test-results from webkit-ews-14

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: <class 'webkitpy.common.config.ports.MacWK2Port'>  Platform: Mac OS X 10.8.2
Comment 28 Kassy Coan 2013-03-22 04:28:22 PDT
Created attachment 194512 [details]
Patch
Comment 29 Kassy Coan 2013-03-22 06:00:54 PDT
Created attachment 194525 [details]
Patch
Comment 30 Kassy Coan 2013-03-22 06:03:28 PDT
Patch now passing mac-wk2 bot! This patch is to see if we still pass when test expectations is not set to slow.
Comment 31 Build Bot 2013-03-22 08:11:00 PDT
Comment on attachment 194525 [details]
Patch

Attachment 194525 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17120730

New failing tests:
inspector/extensions/extensions-resources.html
Comment 32 Build Bot 2013-03-22 08:11:03 PDT
Created attachment 194549 [details]
Archive of layout-test-results from webkit-ews-12

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: <class 'webkitpy.common.config.ports.MacWK2Port'>  Platform: Mac OS X 10.8.2
Comment 33 Kassy Coan 2013-03-22 17:53:37 PDT
Created attachment 194670 [details]
Patch
Comment 34 WebKit Review Bot 2013-03-22 17:56:28 PDT
Attachment 194670 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/platform/mac-wk2/TestExpectations', u'Source/WebCore/ChangeLog', u'Source/WebCore/css/CSSParser.cpp', u'Source/WebCore/css/CSSParserMode.h', u'Source/WebCore/page/FeatureObserver.cpp', u'Source/WebCore/page/FeatureObserver.h']" exit_code: 1
LayoutTests/platform/mac-wk2/TestExpectations:332:  expecting "[", "#", or end of line instead of "{"  [test/expectations] [5]
LayoutTests/platform/mac-wk2/TestExpectations:332:  Path does not exist.  [test/expectations] [5]
Total errors found: 2 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 35 Kassy Coan 2013-03-22 18:04:30 PDT
Created attachment 194674 [details]
Patch
Comment 36 WebKit Review Bot 2013-03-22 20:29:04 PDT
Comment on attachment 194674 [details]
Patch

Attachment 194674 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17232486

New failing tests:
fast/css/font-family-pictograph.html
Comment 37 WebKit Review Bot 2013-03-22 20:29:09 PDT
Created attachment 194686 [details]
Archive of layout-test-results from gce-cr-linux-08 for chromium-linux-x86_64

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-08  Port: chromium-linux-x86_64  Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04
Comment 38 Build Bot 2013-03-23 02:35:25 PDT
Comment on attachment 194674 [details]
Patch

Attachment 194674 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17240373

New failing tests:
inspector/extensions/extensions-resources.html
inspector/styles/get-set-stylesheet-text.html
Comment 39 Build Bot 2013-03-23 02:35:29 PDT
Created attachment 194696 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2
Comment 40 Kassy Coan 2013-03-24 17:37:03 PDT
Created attachment 194774 [details]
Document as RefPtr
Comment 41 Anders Carlsson 2013-09-12 22:52:49 PDT
This bug doesn't have to be open.